Skip to content

Flagging improvements#1766

Merged
ArtOfCode- merged 8 commits intodevelopfrom
0valt/1289/flag-validations
Aug 10, 2025
Merged

Flagging improvements#1766
ArtOfCode- merged 8 commits intodevelopfrom
0valt/1289/flag-validations

Conversation

@Oaphi
Copy link
Member

@Oaphi Oaphi commented Aug 8, 2025

closes #1289

This PR includes the following improvements to flagging:

  • flags are now validated for max length (see linked issue) - configurable via a new site setting MaxFlagReasonLength;
  • flag creation messages have been moved to be provided by the server;
  • we no longer rely on jQuery for raising flags;
  • we now have locale strings for flag actions (Rails must be restarted for it to be picked up) - only flags.errors.rate_limited is added for now, though, to keep the PR simple;

@Oaphi Oaphi requested review from a team, cellio and trichoplax August 8, 2025 15:23
@Oaphi Oaphi added this to the v0.12.3 milestone Aug 8, 2025
@codecov
Copy link

codecov bot commented Aug 8, 2025

Codecov Report

❌ Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 74.89%. Comparing base (2ae9944) to head (4965b39).
⚠️ Report is 18 commits behind head on develop.

Files with missing lines Patch % Lines
app/controllers/flags_controller.rb 75.00% 1 Missing ⚠️
Additional details and impacted files
Components Coverage Δ
controllers 70.03% <75.00%> (+0.17%) ⬆️
helpers 81.82% <ø> (ø)
jobs 60.00% <ø> (ø)
models 86.17% <100.00%> (+0.04%) ⬆️

☔ 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.

Copy link
Member

@cellio cellio left a comment

Choose a reason for hiding this comment

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

Tested with <, =, and > limit and works as expected. Most of the code makes sense to me (do get a more qualified reviewer), though the ...flag part on line 458 in the JS file I don't understand at all (my failing not yours). I left a couple comments asking about ways to improve the user feedback; if the answer is "not now" and that would be a separate issue later, that's fine. But if it's easy, can we try to do something here?

Copy link
Member

@cellio cellio left a comment

Choose a reason for hiding this comment

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

Approved behavior/tests; someone else should also review.

@ArtOfCode- ArtOfCode- merged commit 0103086 into develop Aug 10, 2025
19 of 20 checks passed
@ArtOfCode- ArtOfCode- deleted the 0valt/1289/flag-validations branch August 10, 2025 19:45
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.

Flag message length is not validated at all

3 participants