Repository navigation
Conversation
The readonly-attribute cause was the one plain Hash reaching the service, so cause merging had to guard with try. With every cause a CauseOfChange::Base, the service calls merge_into directly and an untyped cause fails loudly.
Attribute changes are keyed by column, so the readonly check never matched "author" and an admin setting only the author got no cause on the journal.
akabiru
changed the title
github-actions
Bot
added
the
ai: Collaborative 💻
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket
https://community.openproject.org/wp/COMMS-1067
What are you trying to accomplish?
The journal service now receives only cause objects. Merging a label deletion into the previous journal's cause (#26191) needed a
try, because one caller still handed the service a plain Hash: the "Read-only attributes written" cause set when an admin writescreated_ator the author through the API. That cause is now its own class, so the service callsmerge_intodirectly, and a caller passing anything else fails loudly instead of silently never merging.The same check also never fired for an admin who set only the author, since attribute changes are keyed by column (
author_id), not association name. It now does.What approach did you choose and why?
Fixing the one producer keeps the service's contract honest without adding a coercion layer for a case only tests used. Saved journal data is unchanged: a cause object serialises to the same JSON as the Hash did, and comparing a stored cause Hash with a cause object still matches. The acts_as_journalized specs that passed bare strings or Hashes as causes now pass real cause types.
AI involvement
Collaborative – AI generated a substantial part of the code; I reviewed and understand every line.
Merge checklist