Skip to content

Fix for 1923 movement tracking error#1939

Open
poplarmedia wants to merge 1 commit into
collectiveaccess:dev/2.0from
poplarmedia:fix/1923-movement-tracking
Open

Fix for 1923 movement tracking error#1939
poplarmedia wants to merge 1 commit into
collectiveaccess:dev/2.0from
poplarmedia:fix/1923-movement-tracking

Conversation

@poplarmedia

@poplarmedia poplarmedia commented Jun 18, 2026

Copy link
Copy Markdown

Fixes #1923
Root cause analysis by @Monica-Wood in the issue thread. This PR implements fixes based on her findings.
There are two edits:

  • app/models/ca_storage_locations.php — fixes $new_parent_id to read from the request rather than the model, and moves the guard check to after when the form data is available
  • app/lib/BundlableLabelableBaseModelWithAttributes.php — prevents type_id when set via code from being overwritten by the request

@Monica-Wood

Copy link
Copy Markdown
Contributor

You should really link back to the issue that this is trying to fix, in this case issue #1923.
And if you use someone else's work in a fix, you should at least reference them.

Not sure you are aware, but you didn't rebase against dev/2.0 before you submitted and it is undoing previous commits in the process, you need to fix that before it can even be considered.

@poplarmedia
poplarmedia force-pushed the fix/1923-movement-tracking branch from 825257d to 176b3b0 Compare June 25, 2026 01:37
@poplarmedia
poplarmedia force-pushed the fix/1923-movement-tracking branch from 176b3b0 to 69d7b0e Compare June 25, 2026 01:46
@poplarmedia

Copy link
Copy Markdown
Author

@Monica-Wood thank you for double checking my work, and catching the missed rebase. It should be fixed now. My apologies for not crediting your original research in the PR that helped me narrow the problem. That has been addressed as well, as has the ticket link.

@poplarmedia poplarmedia changed the title Fix/1923 movement tracking Fix for 1923 movement tracking error Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants