Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TAN-3658] Add new not_reactable_status_code reason to deny reacting_idea #10197

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

jinjagit
Copy link
Contributor

@jinjagit jinjagit commented Jan 28, 2025

BE code only, at this point.

Changelog

Technical

  • [TAN-3658] Add new not_reactable_status_code reason to deny reacting_idea. To be used to prevent voting on a proposal with prescreening, expired or ineligible status (including in mobile view)

@jinjagit jinjagit self-assigned this Jan 28, 2025
Copy link

@cl-dev-bot
Copy link
Collaborator

cl-dev-bot commented Jan 28, 2025

Messages
📖 Changelog provided 🎉
📖 Notion issue: TAN-3658
📖

Run the e2e tests

📖 Check translation progress

Generated by 🚫 dangerJS against e282e88

@brentguf
Copy link
Contributor

brentguf commented Feb 3, 2025

@jinjagit You can revert the FE changes in this PR. I'll open a separate PR.

Copy link
Contributor

@jamesspeake jamesspeake left a comment

Choose a reason for hiding this comment

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

Looks generally good. It's not necessarily an issue but the fact that the number of queries has gone from 3 to 8 maybe warrants looking at some more

@jinjagit
Copy link
Contributor Author

jinjagit commented Feb 3, 2025

@brentguf

You can revert the FE changes in this PR. I'll open a separate PR.

Done. Thanks!

@jinjagit jinjagit removed the request for review from brentguf February 3, 2025 12:49
@jinjagit jinjagit changed the title [TAN-3658] Prevent voting on proposal with expired or ineligible statuses [TAN-3658] Add new not_reactable_status_code reason to deny reacting_idea Feb 4, 2025
@@ -19,6 +19,7 @@ class IdeaStatus < ApplicationRecord
CODES = %w[prescreening proposed threshold_reached expired viewed under_consideration accepted implemented rejected answered ineligible custom].freeze
LOCKED_CODES = %w[prescreening proposed threshold_reached expired].freeze
MANUAL_TRANSITION_NOT_ALLOWED_CODES = %w[prescreening threshold_reached expired].freeze
REACTING_NOT_ALLOWED_CODES = %w[expired ineligible].freeze
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we include prescreening?

Copy link
Contributor Author

@jinjagit jinjagit Feb 4, 2025

Choose a reason for hiding this comment

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

I've added it in, but it might be YAGNI as an idea with a prescreening status can't have a publication_status: 'published', so it's unlikely anyone (other than admins, mods & author) can see the idea to vote on it.

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.

4 participants