Skip to content

fix(route_manager history): move history update to after model save#160

Merged
crankynetman merged 1 commit intomainfrom
fix/mysterious500s-on-old-entries
Jul 7, 2025
Merged

fix(route_manager history): move history update to after model save#160
crankynetman merged 1 commit intomainfrom
fix/mysterious500s-on-old-entries

Conversation

@crankynetman
Copy link
Copy Markdown
Collaborator

@crankynetman crankynetman commented Jul 7, 2025

We need to actually update the Entry change reason after we save the Entry model which then creates a history model for that change. Before, we were doing it in the serializer before the model is saved, which means that the query to the DB that looks for the appropriate historical_entry instance doesn't find anything because it's looking for the historical entry that hasn't been saved yet.

This is related to #138 and actually, I think, would have fixed it, but we came at it from a different (and frankly, I guess, wrong) angle. Because update_change_reason only edits a historical entry, not create a new one, but I was originally assuming that this function call was what created the historical entry in the first place. In #138 we "fixed" this issue by making sure that the database migrations went and edited all the history to have the default value which then means we technically were editing the comment for the wrong history entry.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jul 7, 2025

File Coverage
All files 83%
config/consumers.py 78%
config/urls.py 69%
config/settings/base.py 69%
config/settings/local.py 73%
scram/route_manager/admin.py 85%
scram/route_manager/models.py 70%
scram/route_manager/views.py 88%
scram/route_manager/api/serializers.py 73%
scram/route_manager/api/views.py 83%
scram/shared/shared_code.py 56%
scram/templates/403.html 91%
scram/templates/404.html 91%
scram/templates/base.html 99%

Minimum allowed coverage is 50%

Generated by 🐒 cobertura-action against 1655065

@crankynetman crankynetman marked this pull request as ready for review July 7, 2025 18:16
@crankynetman crankynetman merged commit 422e5cf into main Jul 7, 2025
19 checks passed
@crankynetman crankynetman deleted the fix/mysterious500s-on-old-entries branch July 7, 2025 18:26
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