Skip to content

Save+share using public ids instead of tokens - #415

Merged
raphaellaude merged 81 commits into
mainfrom
save-share-public-ids
Aug 6, 2025
Merged

Save+share using public ids instead of tokens#415
raphaellaude merged 81 commits into
mainfrom
save-share-public-ids

Conversation

@nofurtherinformation

@nofurtherinformation nofurtherinformation commented Jul 7, 2025

Copy link
Copy Markdown
Collaborator

This PR refactors save/share functionality to incorporate a public sequential ID (accessed when a map is moved off of 'scratch work' or shared with a frozen link. Additionally, it moves toward sharing in one of the three forms:
-map/{public id} - Read only public map
-map/edit/{document id} - Editable map
-map/{public id}?pw=true - Password required share link

Reviewers

Checklist

  • Creating a map takes you to /map/edit/foo
  • Sharing a link frozen yields /map/{public id}
  • Sharing a link with password yields /map/{public id}?pw=true
  • Password unlocking a pw link redirects to the edit ID
  • Public maps can be copied
  • In-use (locked/checked out maps can be copied)
  • Thumbnails generate from the edit ID and generate to the public ID

Screenshots (if applicable):

@nofurtherinformation
nofurtherinformation marked this pull request as ready for review August 5, 2025 02:22
@nofurtherinformation

nofurtherinformation commented Aug 5, 2025

Copy link
Copy Markdown
Collaborator Author

I think this is ready for review. Some key updates:

  • The assignment reload on tab focus is now resolved; tanstack had a behavior to auto re-fetch on focus which caused the behavior and has been flagged to not refetch unless we do it manually
  • The refocus UX has been further cleaned up with some additional logic to prevent flashing of progress indicators
  • Tab refocus rendering has some specific handling for the case where the browser tab goes to sleep and loses GL context for some weird reason; the app does a check to see if maplibre state is correct and if not do a full re-render
  • Metadata now comes from the mapDocument rather than userMaps, which caused public views to not have the correct metadata
  • Custom cursor icons are fixed, this was a filepath issue
  • I removed the publish requirement for now. I think this leads to some tricky edge cases and may not ultimately be better UX
  • Thumbnails on the preview are being fussy, but I think this is just an environment issue uploading to the wrong s3 bucket
  • Added better error text when going to a document that doesn't exist
  • Contiguity and unassigned endpoints now use the public ID

@raphaellaude raphaellaude left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looking good. A few comments:

  • H: Why make documents accessible publicly if they aren't ready to share?
  • H: I'm getting invalid password on checkout. We should add the tests back in to make sure this endpoint works
  • Saving the password collapses the text field and the share type goes back to frozen, so you have to click copy link while the PW dialog is open.
Image
  • After getting the 404 doc not found error (which looks great!) we should probably route the user back to the blank map rather than leave them hanging on the /edit/ page
Image

Comment thread backend/app/core/dependencies.py Outdated
Comment on lines +138 to +136
stmt = stmt.where(Document.public_id == document_id.value).where(
text("map_metadata->>'draft_status' = 'ready_to_share'")
)
stmt = stmt.where(Document.public_id == document_id.value)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

H: why remove this? do we want all maps to be publicly accessible before they are ready to share?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think the case of someone sharing the map while not finished is more likely than the case of someone exploring random numbers and happening across an unfinished plan

but not 100% sure if this is the desired functionality
@nofurtherinformation

Copy link
Copy Markdown
Collaborator Author
  • H: I'm getting invalid password on checkout. We should add the tests back in to make sure this endpoint works

Chris and I experienced this, too. I'll take a look today and see what's going on!

@nofurtherinformation

Copy link
Copy Markdown
Collaborator Author

Good catches on the PW field collapsing and redirect -- I'll update those today

@nofurtherinformation

Copy link
Copy Markdown
Collaborator Author

redirect

Resolved in ca3b963

PW field collapsing

Resolved in 5f1cbad

invalid password on checkout

I believe this is resolved in 0dd4c9e but we should add some more testing

@raphaellaude raphaellaude left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Lgtm! Nice work

@raphaellaude

Copy link
Copy Markdown
Collaborator

Lgtm! Nice work

going to merge to unblock some downstream work

@raphaellaude
raphaellaude merged commit caf8806 into main Aug 6, 2025
2 checks passed
@raphaellaude
raphaellaude deleted the save-share-public-ids branch August 6, 2025 08:30
fangge518 added a commit that referenced this pull request Jul 30, 2026
- mapStore.ts: initialLoadingState only checked the district route
  prefix, a leftover from before community mode existed (PR #415,
  pre-COI) that never got extended when COI shipped. COI page loads
  started INITIALIZING instead of LOADING, so setMapRef could mark
  the app LOADED before the real document ever arrived.
- pageMetadataUtils.ts: generateMapPageMetadata read the document id
  from searchParams, but every id-bearing map/coi route now (post
  URL-flip) carries it as a path param instead — so district map/edit
  /eval pages were silently generating generic OG metadata (broken
  "Map not found" preview image) instead of the map's own name/
  description/thumbnail. Now reads params.public_id first, falling
  back to searchParams.document_id only for the legacy /map?document_id=
  redirect landing page. Dropped the COI pages' now-redundant manual
  searchParams repackaging now that direct assignment works for them
  too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants