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.

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

Should we always mint a new ID when creating a tree or study? #1302

Open
jimallman opened this issue Feb 14, 2023 · 5 comments
Open

Should we always mint a new ID when creating a tree or study? #1302

jimallman opened this issue Feb 14, 2023 · 5 comments
Assignees

Comments

@jimallman
Copy link
Member

This might avoid confusion in our tree-collection feature, if it can mis-match a new study/tree because serial IDs are being re-used. (E.g., If I remove tree2 from a study, then create a new tree, does it re-use the ID?)

IF this is happening, we should save the last-used IDs for each and ALWAYS increment.

@jimallman jimallman self-assigned this Feb 14, 2023
@jimallman
Copy link
Member Author

OK, so we do this properly for new studies. Each is issued a unique, newly-minted ID, and a persistent counter is incremented. So all good there. I'll check tree IDs next.

@jimallman
Copy link
Member Author

OK, so new tree IDs are minted in client-side Javascript. During a single study-edit session, we liberally mint new IDs for each attempted submission, so there's no chance of a duplicate. BUT we don't save this elementTypes info with the study, so the same ID (upload failed, or not saved) could be re-used in a later session. This could happen because when the curation tool loads an existing study, it checks all existing IDs, finds the highest number (e.g. tree-6) and prepares to mint the next logical ID (tree-7).

It's kind of an edge case, but if the consequences are dire enough (wrong tree in synthesis!) we could manage this by saving the last-attempted tree ID in the study Nexson, or by minting stronger unique IDs along the lines of tree-2342423. Thoughts?

jimallman added a commit that referenced this issue Feb 20, 2023
This should avoid possible confusion, e.g. when adding trees to a collection.
We want to avoid using `tree4` in a collection, then deleting it and
re-using the old ID. Addresses #1302.
@jimallman
Copy link
Member Author

@snacktavish The feature branch above (currently working on devtree) mints new tree IDs by a completely different method. Instead of simple incrementing IDs (tree1, tree2...) it tries to make each one unique in the current studio, but also unlikely (tree478290) to handle a sequence like

  • add/edit tree4
  • add to a tree collection
  • delete tree4 from study
  • add new tree4 to study
  • collection now contains an unintended tree

The only safe alternative I can think of would be for each study to keep track of all IDs it has used or attempted to use previously. I suppose keeping a old-school counter would work, but still fails in the event of a tree that's created, added to a collection, then the curation session is abandoned without saving. So we're back to needing to force a save of the study before adding any of its trees to a collection. Seems weird, but maybe the lesser of evils if we don't like this.

@jimallman
Copy link
Member Author

@snacktavish Oh, one minor usability query: Would these six-digit tree IDs like tree875920 just make it painful to talk about trees? I mean, tree2 is at least easy to say and find. Maybe there's a still-unlikely compromise, like tree4839?

@snacktavish
Copy link
Member

I'll take a look, but I def think single digits are highly preferable, because there are several places where a human needs to read and track them. Maybe even worth the readability at the risk of tree number re-use.

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

No branches or pull requests

2 participants