-
Notifications
You must be signed in to change notification settings - Fork 36
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
MOTs business logic: Clear CO form when CO Type changes #5100
Conversation
TODO:
|
This reverts commit 7dba89d.
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.
Are we clearing all fields on the CollectionObject?
What I gathered from #4640 (comment) is that only the CollectionObjectAttribute fields should be cleared.
Other fields on CollectionObject can remain untouched.
Is this misunderstanding the requirements, or have the requirements changed?
(If we go with a businessrule approach for determing the Taxon tree to search on for the Determination -> QueryComboBox and the current implementation style of #5127, then Determinations should be cleared as well).
Hmm, looks like #4640 mentions only clearing determinations and collectionObjectAttribute fields - which would make this a lot simpler. However, I think we discussed in a meeting that we should clear all fields as we don't know what fields are gonna be conditionally rendered - which should handle all possibilities. Thoughts on this? @grantfitzsimmons |
Yes that is correct |
@sharadsw The decision for now is to clear everything– I just spoke with Aimee and it seems like the simplest solution for the MVP. I feel that we should only clear fields not visible on the form (in case of conditionals) and the determination, but maybe that is something we revisit later. |
Triggered by ab44ebc on branch refs/heads/issue-5088b
Trying to clear all to-many dependent fields with the simple Right now this PR will clear all simple fields and independent relations and also delete all determinations added in the CO form whenever CollectionObjectType changes. Does this sound good? @grantfitzsimmons |
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.
Testing the PR
- Go to Data Entry -> Collection Object
- Enter some determinations
- Enter data in different basic fields (text/number fields)
- Change CollectionObjectType value
- Verify all simple fields are cleared
- Verify any added determinations are deleted
- Re-run the test for an already existing Collection Object
Looks good, all fields expected to clear were cleared
That sounds good to me. |
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.
Testing the PR
- Go to Data Entry -> Collection Object
- Enter some determinations
- Enter data in different basic fields (text/number fields)
- Change CollectionObjectType value
- Verify all simple fields are cleared
- Verify any added determinations are deleted
- Re-run the test for an already existing Collection Object
All fields I tried were cleared 👍👍👍
chrome_uq7cIGOCjt.mp4
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.
Testing the PR
- Go to Data Entry -> Collection Object
- Enter some determinations
- Enter data in different basic fields (text/number fields)
- Change CollectionObjectType value
- Verify all simple fields are cleared
- Verify any added determinations are deleted
- Re-run the test for an already existing Collection Object
Looks good, all fields cleared on new and pre-existing COs👌
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.
This is fine given it is acceptable for the requirements and a temporary solution or fallback if a more robust solution can not be implemented by the deadline.
Not a fan of clearing all literal fields and some relationships here.
Either of the previously discussed solutions (only clearing fields used in a conditional panel or having separate view definitions based on CollectionObjectType) will be well worth the complexity, and far better from a resultant UX standpoint.
Complexity does take time, and that we don't have much of, so something is better than nothing.
Still, this is a fine start while we work out everything else for Geo.
Onto the next fish to fry! 👏
This pull request has been mentioned on Specify Community Forum. There might be relevant details there: https://discourse.specifysoftware.org/t/specify-7-9-7-release-announcement/1979/1 |
Fixes #5088
Using business rules to clear CO values when COType changes. Needs #5098 to be fixed to test with configuring a pick list in Schema Config. Until then, it can be tested with CoType as a QCBX.
The following fields are ignored when clearing for better UX or based on backend restrictions (for eg: backend needs a non-null value for version)
specify7/specifyweb/frontend/js_src/lib/components/DataModel/businessRuleDefs.ts
Lines 155 to 161 in 64fd7ea
Checklist
and self-explanatory (or properly documented)
Testing instructions
Set up a QCBX for CollectionObjectType (like in #5127):
Ideally, COType would be set as a picklist. However, until #5098 is fixed, it is not possible to configure one for COType. Instead for this PR, COType can be set up as a QCBX.
.views
file for the collection you are in<row>
:Your CO form should look something like this
Testing the PR