-
Notifications
You must be signed in to change notification settings - Fork 37
Create a new component table for CO children #6721
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
base: main
Are you sure you want to change the base?
Conversation
Definitely! Anytime catalog number is inherited |
Not an Issue with this PR, just a quick note here for
I tracked down the cause of the consistent migration error, and right now it will happen on any database moving from Specifically, the error is completely correct: the The
When the changes in The change was introduced in 9bf24fa; PR #6885. The result is that whenever someone goes from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work so far! 🥳
This is still only a partial review (I'm still reviewing the rest!), but I will submit this review so the points can be addressed/discussed whilst I wrap up looking over the rest of the code/tests.
specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Jason Melton <[email protected]>
Co-authored-by: Jason Melton <[email protected]>
Avoid 0036_remove_spquery_selectseries errors on test-panel
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on the frontend changes! It's coming together 👏
This review covers what is left of the backend from #6721 (review).
Most of the comments are related to the migration itself, but there are a few comments for other parts as well.
I do also have some questions regarding the catalogNumberParentInheritance
Collection Preference.
What is the preference trying to accomplish and why is it needed? I could not easily find where the requirements for the preference are. If you know where they are, could you provide a link or summary in a comment or the PR description?
Primarily my question is that: instead of showing the parent catalogNumber as a placeholder (and keeping the Component -> catalogNumber as empty/null) and modifying queries using Component -> catalogNumber -> Equal, what is the issue with literally inheriting the parent's catalogNumber? So whenever a Component is added to a CollectionObject, the Component's catalogNumber would be set to the CollectionObject's1.
That way, we would not need to modify any query behind the scenes, and a user constructing a query could leverage all of the other filters (IN
, LIKE
, CONTAINS
, etc.).
Currently, there is nothing wrong with having Components with duplicate catalogNumbers (if there is, we should provide a default Uniqueness Rule).
If we are keeping the current placeholder solution, could we at least show the placeholder for new Components as well? It might be confusing that there is a difference for new/existing Components.
Screen.Recording.2025-07-16.at.12.41.29.PM.mov
Some other general comments:
- Are we providing other defaults for Components? Things like:
- Record Formatter
- Record Aggregator
- TypeSearch
- Field Format (for CatalogNumber field?)
- Uniqueness Rules
- I would love to see a visual editor for Collection Preferences! Even if Statistics preferences wouldn't yet be included, just for these and other simple preferences.
Even with instruction, manually editing a busy non-formatted JSON file is intimidating and prone to error
Footnotes
-
we would need to delay doing so for new COs which are auto-incremented, at least until the CO is saved and then we can set the Components catalogNumber ↩
Co-authored-by: Jason Melton <[email protected]>
Co-authored-by: Jason Melton <[email protected]>
Co-authored-by: Jason Melton <[email protected]>
Co-authored-by: Jason Melton <[email protected]>
Co-authored-by: Jason Melton <[email protected]>
Co-authored-by: Jason Melton <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@CarolineDenis After reviewing some more, we are missing a critical field which I did not consider previously – determiner
, which is necessary to say who decided a component was of a particular taxonomy. Can this be added? The original requirements have already been updated.
Fixes #6708
Warning
This PR affects database migrations. See migration testing instructions.
Checklist
self-explanatory (or properly documented)
Testing instructions
1. Initial Setup
2. Schema Configuration
3. Data Entry: Component with its Own Catalog Number
4. Data Entry: Component with Catalog Number Inherited from CO
5. Assigning Taxon Name to Component
6. Autocomplete Filtering by Type
7. Validation on Mismatched Type and Name Trees
8. Query Builder: Component Search
9. Query Builder: Component with Inherited Catalog Number
10. Workbench: Component Table as Base
11. Workbench: CO Table as Base
CO catalog number
Component