Skip to content

Fix tag drafts not being deleted upon creation / update#1867

Merged
Oaphi merged 9 commits intodevelopfrom
0valt/1859/tag-drafts
Oct 6, 2025
Merged

Fix tag drafts not being deleted upon creation / update#1867
Oaphi merged 9 commits intodevelopfrom
0valt/1859/tag-drafts

Conversation

@Oaphi
Copy link
Member

@Oaphi Oaphi commented Oct 5, 2025

closes #1859

This PR adds a DraftManagement concern to both facilitate the fix and avoid duplication (all our controllers share the same draft saving & deletion logic). The concern is named this way to match Rails's convention of naming such controller concerns (see, for example, RequestForgeryProtection)

@codecov
Copy link

codecov bot commented Oct 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.84%. Comparing base (7f107ed) to head (c01a33e).
⚠️ Report is 10 commits behind head on develop.

Additional details and impacted files
Components Coverage Δ
controllers 72.74% <100.00%> (+0.47%) ⬆️
helpers 84.16% <ø> (ø)
jobs 60.86% <ø> (ø)
models 89.23% <100.00%> (+0.01%) ⬆️
tasks 61.11% <ø> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Oaphi Oaphi added this to the v0.12.4 milestone Oct 5, 2025
Comment on lines +118 to +120
@controller.stub('params', { tag_name: tag_name }) do
@controller.do_save_draft(user, draft_path)
end
Copy link
Member Author

@Oaphi Oaphi Oct 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an unfortunate mess, but doing this "the proper way" would require quite a bit of work that's outside of the scope of the PR (although I am not against making these changes if necessary): move all this to integration tests, move save_draft and delete_draft to concern's actions, etc.

# @param tag_set [TagSet] tag set the tag belongs to
def try_create_tag(tag_set, category, **opts)
# this is needed for draft deletion until we switch from request.referer
@request.set_header('HTTP_REFERER', create_tag_path(id: category.id))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this is only here because we rely on request.referer when creating and deleting drafts. I'd like to work on switching from that approach separately, but open to doing it "while we are at it" too.

@Oaphi Oaphi requested review from ArtOfCode- and cellio October 5, 2025 17:46
@Oaphi Oaphi marked this pull request as ready for review October 5, 2025 17:46
@Oaphi Oaphi merged commit 6f1474a into develop Oct 6, 2025
13 checks passed
@Oaphi Oaphi deleted the 0valt/1859/tag-drafts branch October 6, 2025 19:22
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.

Drafts are not cleared after creating tags

2 participants