Skip to content
This repository has been archived by the owner on Jun 10, 2024. It is now read-only.

fix: remove uid from scenes #274

Merged
merged 6 commits into from
Sep 8, 2023
Merged

fix: remove uid from scenes #274

merged 6 commits into from
Sep 8, 2023

Conversation

rostyslavvnahornyi
Copy link
Collaborator

#issue260
Hello! @kfarr, this pr includes issue#247, issue#248, issue#260.

P.S.: if you have got old scenes in firestore, delete them or just delete uid fields.

when running on db with permissions set to only allow update operations from the author of a document, the initially generated document must set author
@kfarr
Copy link
Contributor

kfarr commented Sep 7, 2023

Here are some WIP notes:

  • We use the built-in firestore ID field but I added back using UUID as the actual ID, working great
  • There was an issue related to author permissions that I resolved. @rostyslavvnahornyi I'd suggest that you use the same firestore security rules as here on your development database so that you can catch these errors: https://github.com/3DStreet/3dstreet-editor/blob/feature-firebase-save-load/public/firestore.rules
  • The new timestamp fields are generated correctly when saving a new file
  • entity?.object3D?.traverse((node) => { adding the "?" operator -- chat gpt tells me this code change will check for existence of entity and object3d before attempting to traverse. I might revert to original so that we get the actual error and can fix the root cause instead of silently ignoring this way
  • Save (existing file) appears to work as expected

To do before merging:

  • remove logic to open new window when saving scene
  • make different notification success messages for creating newly saved doc vs saving existing doc
  • errors captured from try/except in uploadScene (scene.js) should be exposed as notification message as well as console message
  • try to "break" the hash logic -- see what happens with invalid uuid passed, for example
  • NOT DOING, will let it break instead - update dev DB to retain some of the existing test scenes in new schema format (not doing)
  • new issue for later, but ok to let go for this PR - as user who opens a scene, then opens sidebar menu, save as.. is used instead of save #275
  • confirm "save as" ticket given above

* fix setdoc import mistake
* change name uploadScene to updateScene
* add savedNewDocument state to toolbar to handle save vs. save as notification
* change name from uploadSceneHandler to cloudSaveHandler
* change name from getCurrentSceneUuid to getSceneUuidFromURLHash
* change logic to fetch documentId from state if exists before referring to URL
* remove open new window
* add notification for save vs. save as...
@kfarr kfarr merged commit 083d31a into feature-firebase-save-load Sep 8, 2023
@kfarr kfarr deleted the issue#260 branch September 8, 2023 04:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants