Fix checksum update on retiring names#885
Conversation
…sion creation * Introduce static method to detect retired status edits not affecting checksum * Update version cloning logic to consider locale status changes via * Add integration test verifying PUT returns 200 when only name retired changes * Ensure proper handling of active vs retired name transitions in versioning
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e40eef21a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| mnemonic=instance.mnemonic, parent_id=instance.parent_id, is_latest_version=True | ||
| ).first() | ||
| names_payload = data.get('names', []) | ||
| descriptions_payload = data.get('descriptions', []) |
There was a problem hiding this comment.
When a concept update sends "descriptions": null (explicitly accepted by ConceptDetailSerializer.descriptions with allow_null=True), this line stores None; if no name status change short-circuits first, the new call to _locale_status_changed passes that None into zip(new_locales, locale_payloads) and raises TypeError: 'NoneType' object is not iterable. Existing build logic treats None as an empty description list, so normalize this payload with or [] before the new status-change check.
Useful? React with 👍 / 👎.
* Introduce ChecksumBase subclass to include locale retirement fields in standard checksum * Update tests to verify checksum variations when locale status changes * Remove unused _locale_status_changed method from Concept model
…status fields * Replace ChecksumBase with OCLAPIChecksum throughout core * Introduce LOCALE_STATUS_CHECKSUM_FIELDS constant for retired/status tracking * Update checksum generation calls in ChecksumModel and related methods * Adjust tests to import OCLAPIChecksum and use the new class name * Update patch decorator target for checksum generation mocks�
* Update import statements from OCLAPIChecksum to ChecksumBase * Change class inheritance from OCLAPIChecksum to ChecksumBase * Adjust test code and patches to use ChecksumBase
* remove unused ChecksumBase subclass and related methods * simplify checksum import to use public API * bump ocldev dependency to 0.2.4�
| self.assertEqual(response.data['display_name'], prev_version.display_name) | ||
| self.assertEqual(concept.datatype, "N/A") | ||
|
|
||
| def test_put_200_when_name_retired_changes_only(self): |
There was a problem hiding this comment.
@filiperochalopes This test passes on master with ocldev==0.2.3 as well.
2 participants
Solves OpenConceptLab/ocl_issues#2583
Depends on OpenConceptLab/ocldev#18