Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't send geometry Ids to the frontend #5217

Merged
merged 1 commit into from
Jan 7, 2025
Merged

Don't send geometry Ids to the frontend #5217

merged 1 commit into from
Jan 7, 2025

Conversation

jkeiser
Copy link
Contributor

@jkeiser jkeiser commented Jan 7, 2025 •
edited
Loading

We are overwriting the view ID with the geometry ID in some cases.

This made it so that if you added a link to a view and then merged to head, you would get errors (@keeb was seeing them).

W/ @vbustamante and @jobelenus

@github-actions GitHub Actions
Copy link

github-actions bot commented Jan 7, 2025 •
edited
Loading

Dependency Review

✅ No vulnerabilities or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Files

@jkeiser jkeiser force-pushed the jkeiser/view-geometry-id branch from aabfb2a to 835e73a Compare January 7, 2025 21:30
@github-actions github-actions bot added A-sdf Area: Primary backend API service [Rust] A-web labels Jan 7, 2025
@vbustamante vbustamante changed the title Don't send Don't send geometry Ids to the frontend Jan 7, 2025
jobelenus
jobelenus previously requested changes Jan 7, 2025
View reviewed changes
Copy link
Contributor

@jobelenus jobelenus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need that id in the FE type, since it doesn't come over the wire

app/web/src/store/views.store.ts Outdated Show resolved Hide resolved
@jkeiser jkeiser force-pushed the jkeiser/view-geometry-id branch from 835e73a to 66a66de Compare January 7, 2025 21:59
@jkeiser jkeiser force-pushed the jkeiser/view-geometry-id branch from 66a66de to ad16d1f Compare January 7, 2025 22:01
@vbustamante vbustamante added this pull request to the merge queue Jan 7, 2025
Merged via the queue into main with commit 7a35fa2 Jan 7, 2025
10 checks passed
@vbustamante vbustamante deleted the jkeiser/view-geometry-id branch January 7, 2025 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vbustamante vbustamante vbustamante approved these changes

@jobelenus jobelenus jobelenus approved these changes

Assignees
No one assigned
Labels
A-sdf Area: Primary backend API service [Rust] A-web
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jkeiser @jobelenus @vbustamante