Fix CollectionObject business rule crash on missing CollectionObjectType

[acwhite211]

Fixes #7870

Saving a CollectionObject with a missing collectionObjectType could trigger a front-end business rule crash and leave the record stuck on the "Loading..." dialog instead of completing the save. This affected older records, including records created in Specify 6, where CollectionObjectTypeID may still be null.

Added a null guard in the CollectionObject collectionObjectType business rule and skip determination taxon tree validation when no collection object type is set. Clear any existing determination taxon save blockers when collectionObjectType is missing. Also added unit tests for saving when collectionObjectType is null and clearing invalid determination blockers when collectionObjectType becomes null

On the back-end, I decided to go ahead and opportunistically normalize legacy null collectionObjectType values when an existing CollectionObject is saved in Specify 7. If the collection already has a default Collection Object Type, that value is applied on save. If both the record and the collection default are null, Specify creates or reuses the collection’s discipline-based default type, assigns it to the collection, and uses it for the saved record. I'm ok with reverting this section if anyone thinks we should leave them as null.

Checklist

• Self-review the PR after opening it to make sure the changes look good and
  self-explanatory (or properly documented)
• Add relevant issue to release milestone
• Add pr to documentation list
• Add automated tests

Testing instructions

• Open a database that contains at least one CollectionObject with no Collection Object Type set (CollectionObjectTypeID = null). Older Specify 6-created records are the main target.
• Use the CO QB to find a CO record without a COT. If you can't find one, might need to go into the database to set one of them to null. Might be able to set a COT to null in batch edit.

• In Data Entry, open one of those records and confirm the Collection Object Type field is blank.
• Add a new Determination to the record.
• Click Save.
• Verify the save completes normally instead of hanging on the "Loading..." dialog.
• Verify the page remains usable after save and no infinite loading state appears.
• Refresh or reopen the record and confirm the new determination was actually saved.
• Run the same CO QB query to make sure that the COT is not null anymore.

Summary by CodeRabbit

• Bug Fixes

  • Collection object types are now reliably populated when saving determinations or objects, including creating or applying fallback defaults when a collection lacks a configured type.
  • Improved handling of null collection object types to avoid spurious validation/save blockers on dependent determinations, preventing needless save errors.

• Tests

  • Added tests covering defaulting behavior and determination interactions with null or missing collection object types.

[bhumikaguptaa]

• Verify the save completes normally instead of hanging on the "Loading..." dialog.
• Verify the page remains usable after save and no infinite loading state appears.
• Refresh or reopen the record and confirm the new determination was actually saved.
• Run the same CO QB query to make sure that the COT is not null anymore.

Everything looks good, except when i refresh the page and rerun the query, the COT is still null.
Db:https://kuentoissue7870-issue-7870.test.specifysystems.org/specify/query/new/collectionobject/

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 547a4f0c-c088-42b5-b4d9-812f8d56aae9

📥 Commits

Reviewing files that changed from the base of the PR and between f01f2de and fb69d45.

📒 Files selected for processing (1)

• specifyweb/specify/api/utils.py

---

📝 Walkthrough

Walkthrough

Adds helpers to derive/create a default Collectionobjecttype and a function to ensure a Collectionobject has one; calls this from Collectionobject and Determination pre-save hooks (Determination persists the resolved type). Adds backend/frontend tests, a frontend null-guard for collectionObjectType, and a minor stored-queries fixture tweak.

Changes

CollectionObject Type Defaulting

Layer / File(s)	Summary
Core utility functions specifyweb/specify/api/utils.py	get_or_create_default_collection_object_type derives/creates a discipline+taxontree default type and updates Collection rows; ensure_collection_object_type resolves a Collectionobject's type and can persist it. get_picklists return fixed to include picklists.
Backend business rule integration specifyweb/backend/businessrules/rules/collectionobject_rules.py, specifyweb/backend/businessrules/rules/determination_rules.py	Pre-save hooks call ensure_collection_object_type with appropriate DB alias (Determination uses persist=True) before running remaining rule logic; minor formatting change to a BusinessRuleException message line.
Backend test coverage specifyweb/backend/businessrules/tests/test_collectionobject.py	Five tests added verifying behavior when collection/default types are present or absent, fallback creation of discipline-based defaults, and Determination saves populating collectionobjecttype.
Frontend null-guard specifyweb/frontend/js_src/lib/components/DataModel/businessRuleDefs.ts	Adds early null-guard for collectionObjectType field check: when coType is null, set save blockers for dependent determination taxon fields and return early to avoid TypeError.
Frontend test coverage specifyweb/frontend/js_src/lib/components/DataModel/__tests__/businessRules.test.ts	Two Jest tests added to confirm no hang/save-blocker issues when collectionObjectType is null and that blockers clear appropriately.
Stored-queries fixture specifyweb/backend/stored_queries/tests/tests_legacy.py	Removed a "not_found": ["projects"] entry from the expected_errors["Collection"] fixture.

Suggested reviewers

• bhumikaguptaa
• emenslin

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Testing Instructions	⚠️ Warning	Instructions lack procedural details (how to create test data), don't mention verifying console errors, and omit regression testing with non-null COT records.	Add setup steps for null COT records, explicit error verification, regression testing guidance, and explain expected COT assignment behavior.

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: fixing a CollectionObject business rule crash when CollectionObjectType is missing.
Linked Issues check	✅ Passed	The PR comprehensively addresses issue #7870's requirements: null-check guard in businessRuleDefs.ts, clearing determination blockers, unit tests, and back-end normalization.
Out of Scope Changes check	✅ Passed	All changes directly address the null CollectionObjectType handling. The test update in tests_legacy.py (removing a projects entry) is a minor cleanup consistent with the PR scope.
Automatic Tests	✅ Passed	PR includes comprehensive automatic tests: 5 backend tests and 2 frontend tests covering null collectionObjectType scenarios, blocker clearing, and determination handling per PR objectives.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch issue-7870

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

specifyweb/backend/businessrules/tests/test_collectionobject.py (1)

62-91: 💤 Low value

Small note on the discipline rename.

Renaming the discipline to "Fallback Discipline" before triggering the helper looks like a workaround for get_or_create(name=discipline.name, ...) colliding with a Collectionobjecttype that the test fixture already created under the original discipline name. The test passes, but it makes the intent a bit opaque — a one-line comment ("rename so the fallback get_or_create actually creates a new COT") would help the next person who touches this.

Also consider asserting on Collectionobjecttype.objects.filter(name="Fallback Discipline", collection=self.collection).count() == 1 to lock in "creates exactly one" behavior and guard against a future regression that double-creates under the race noted in utils.py.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specifyweb/backend/businessrules/tests/test_collectionobject.py` around lines
62 - 91, In
test_existing_collectionobject_without_type_creates_collection_default_on_save:
add a one-line comment explaining why the discipline name is renamed to
"Fallback Discipline" (so get_or_create(name=discipline.name, ...) will create a
new Collectionobjecttype instead of colliding with the fixture), and after
saving and refreshing assert that exactly one Collectionobjecttype exists for
that name and collection (e.g. assert
Collectionobjecttype.objects.filter(name="Fallback Discipline",
collection=self.collection).count() == 1) to guard against duplicate creation;
reference the test method name, the Collectionobjecttype model and the
get_or_create logic in utils.py when making the change.

specifyweb/specify/api/utils.py (1)

65-91: 💤 Low value

Logic for ensure_collection_object_type reads cleanly.

The branching (already-set → collection default → derive-from-discipline) matches the intended fallback chain, and gating the persist ORM update on pk is not None + collectionobjecttype__isnull=True is the right defensive shape so we never clobber a value that another writer has set in the meantime. Nice.

One small polish: when collection_object.pk is None and the collection has no default, the function returns None and the caller relies on Django's normal save path to write the (still-null) field. That's correct given the frontend now null-guards, but a one-line docstring spelling out the three branches and the "may return None" contract would save the next reader a trip through the call sites.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specifyweb/specify/api/utils.py` around lines 65 - 91, Add a one-line
docstring to the ensure_collection_object_type function explaining its
three-branch behavior and return contract: if
collection_object.collectionobjecttype is already set return it; otherwise use
collection.collectionobjecttype as a default; otherwise derive via
get_or_create_default_collection_object_type; the function may return None when
no type can be determined and callers should handle that (persist and pk gating
behavior remains unchanged). Reference ensure_collection_object_type,
collection_object, persist, and get_or_create_default_collection_object_type in
the docstring for clarity.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@specifyweb/specify/api/utils.py`:
- Around line 38-42: The helper that reads collection.discipline (where
discipline_name = collection.discipline.name and taxon_tree_def_id =
collection.discipline.taxontreedef_id) currently returns None silently when
taxon_tree_def_id is null; update that helper in specify/api/utils.py to emit a
visible log warning (e.g., logger.warning) mentioning the collection identifier
and that discipline.taxontreedef_id is missing before returning None so the
missing COT/backfill state is observable; reference the discipline_name,
taxon_tree_def_id and collectionobjecttype variables in the message to aid
debugging.

---

Nitpick comments:
In `@specifyweb/backend/businessrules/tests/test_collectionobject.py`:
- Around line 62-91: In
test_existing_collectionobject_without_type_creates_collection_default_on_save:
add a one-line comment explaining why the discipline name is renamed to
"Fallback Discipline" (so get_or_create(name=discipline.name, ...) will create a
new Collectionobjecttype instead of colliding with the fixture), and after
saving and refreshing assert that exactly one Collectionobjecttype exists for
that name and collection (e.g. assert
Collectionobjecttype.objects.filter(name="Fallback Discipline",
collection=self.collection).count() == 1) to guard against duplicate creation;
reference the test method name, the Collectionobjecttype model and the
get_or_create logic in utils.py when making the change.

In `@specifyweb/specify/api/utils.py`:
- Around line 65-91: Add a one-line docstring to the
ensure_collection_object_type function explaining its three-branch behavior and
return contract: if collection_object.collectionobjecttype is already set return
it; otherwise use collection.collectionobjecttype as a default; otherwise derive
via get_or_create_default_collection_object_type; the function may return None
when no type can be determined and callers should handle that (persist and pk
gating behavior remains unchanged). Reference ensure_collection_object_type,
collection_object, persist, and get_or_create_default_collection_object_type in
the docstring for clarity.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 953818a5-0b81-4763-b70f-9154865b16cc

📥 Commits

Reviewing files that changed from the base of the PR and between 66b1266 and fa9ac44.

📒 Files selected for processing (6)

• specifyweb/backend/businessrules/rules/collectionobject_rules.py
• specifyweb/backend/businessrules/rules/determination_rules.py
• specifyweb/backend/businessrules/tests/test_collectionobject.py
• specifyweb/frontend/js_src/lib/components/DataModel/__tests__/businessRules.test.ts
• specifyweb/frontend/js_src/lib/components/DataModel/businessRuleDefs.ts
• specifyweb/specify/api/utils.py