Skip to content

Implement etiquette validator on meetings#12876

Closed
arpi-tremend wants to merge 18 commits intodecidim:developfrom
i-need-another-coffee:feature/update-meeting-with-empty-body
Closed

Implement etiquette validator on meetings#12876
arpi-tremend wants to merge 18 commits intodecidim:developfrom
i-need-another-coffee:feature/update-meeting-with-empty-body

Conversation

@arpi-tremend
Copy link
Copy Markdown
Contributor

@arpi-tremend arpi-tremend commented May 23, 2024

Meeting can be updated with an empty body

📌 Related Issues

Link your PR to an issue

Testing

  • Enable rich text editor for participants from Organization settings
  • Create a new Meeting
  • Play with the Title and Description fields content
  • Form save should be prevented by the following error messages or combination, depending on the content:
    • can't be blank,
    • must start with a capital letter
    • is using too many capital letters (over 25% of the text)
  • the same behavior should be observable for the Registration term field, if the Registration type is set to 'On this platform'

📷 Screenshots

Description
Screenshot 2024-05-23 133049
Screenshot 2024-05-23 133125
Screenshot 2024-05-23 133436

♥️ Thank you!
b2b2-74e3dff22590)

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

This pull request does not contain a valid label. Please add one of the following labels: ['type: feature', 'type: change', 'type: fix', 'type: removal', 'target: developer-experience', 'type: internal']

@andreslucena
Copy link
Copy Markdown
Member

@arpi-tremend we need to discuss this on @decidim/product - as I mentioned in #12713 (comment) I'm not sure if the proposed solution (extend this validation in Meetings and then everywhere for consistency) is the correct one. The last times that we've talked about this feature we were really hesitant if this was actually useful or if it was a problem (and it should be go away).

@andreslucena andreslucena self-assigned this May 27, 2024
@andreslucena
Copy link
Copy Markdown
Member

@arpi-tremend we need to discuss this on @decidim/product - as I mentioned in #12713 (comment) I'm not sure if the proposed solution (extend this validation in Meetings and then everywhere for consistency) is the correct one. The last times that we've talked about this feature we were really hesitant if this was actually useful or if it was a problem (and it should be go away).

After some discussion with @carolromero, she proposes:

In general, the feedback we have received over time is that the rules are too restrictive, and I confirm this after checking with other members of the community. So I would advocate relaxing the default active validations, and let implementers choose in each context which ones to apply.

To give more detail in how this would look like, this is what we've come up with:

  • Regarding the too_much_caps validation, instead of 25% of caps should kick in for 50%
  • it should work for all the user generated contents: Proposals, Meetings, Debates
  • it should also work for these same models for Admins, so all the authors share the same validations (and pain)
  • it should have an initializer setting: config.enable_etiquette_validator: booelan type. It should also have a secrets/ENV var. As we're adding a new initializer, we should let implementers know through releases notes
  • as these are changes in validations and settings, this should be added as a type: change, and we can't backport it; if we need a fix for some of the issues (like the meetings empty body bug), then we need to handle that as a new PR so it's a fix and we can backport it

@arpi-tremend @alecslupu let me know if you agree with these criteria and if you can work on all of these (or some or none). Thanks!

@andreslucena andreslucena added the type: change PRs that implement a change for an existing feature label Sep 20, 2024
github-actions[bot]
github-actions bot previously approved these changes Sep 20, 2024
@andreslucena
Copy link
Copy Markdown
Member

There are some failing specs that should be fixed if you merge/rebase with develop. Can you do it @arpi-tremend 🙏🏽 ? Thanks

@alecslupu
Copy link
Copy Markdown
Contributor

There are some failing specs that should be fixed if you merge/rebase with develop. Can you do it @arpi-tremend 🙏🏽 ? Thanks

As @arpi-tremend is in my other team, i will take over this one.

@andreslucena
Copy link
Copy Markdown
Member

@alecslupu let me know when this is ready for a review

@alecslupu alecslupu self-assigned this Nov 15, 2024
github-actions[bot]
github-actions bot previously approved these changes Dec 11, 2024
@alecslupu alecslupu marked this pull request as ready for review December 16, 2024 14:28
@alecslupu alecslupu assigned andreslucena and unassigned alecslupu Dec 16, 2024
@andreslucena andreslucena changed the title Update etiquette validator Implement etiquette validator on meetings Dec 18, 2024
validates :registration_type, presence: true
validates :available_slots, numericality: { greater_than_or_equal_to: 0 }, presence: true, if: ->(form) { form.on_this_platform? }
validates :registration_terms, presence: true, if: ->(form) { form.on_this_platform? }
validates :registration_terms, presence: true, etiquette: true, if: ->(form) { form.on_this_platform? }
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
validates :registration_terms, presence: true, etiquette: true, if: ->(form) { form.on_this_platform? }
validates :registration_terms, presence: true, if: ->(form) { form.on_this_platform? }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need to implement the ettiquette validator in the ToS of the meeting

validates :iframe_embed_type, inclusion: { in: Decidim::Meetings::Meeting.participants_iframe_embed_types }
validates :title, presence: true
validates :description, presence: true
validates :title, presence: true, etiquette: true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add a spec for this?

validates :title, presence: true
validates :description, presence: true
validates :title, presence: true, etiquette: true
validates :description, presence: true, etiquette: true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add a spec for this?

@alecslupu
Copy link
Copy Markdown
Contributor

Closing in favor of #12874

@alecslupu alecslupu closed this Jan 26, 2025
@alecslupu alecslupu deleted the feature/update-meeting-with-empty-body branch January 26, 2025 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

module: core module: meetings type: change PRs that implement a change for an existing feature

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants