Skip to content

Add ability to toggle badges by organization#4249

Merged
mrcasals merged 5 commits intomasterfrom
badges_toggle
Oct 9, 2018
Merged

Add ability to toggle badges by organization#4249
mrcasals merged 5 commits intomasterfrom
badges_toggle

Conversation

@josepjaume
Copy link
Copy Markdown
Contributor

@josepjaume josepjaume commented Oct 8, 2018

🎩 What? Why?

This allows each organization to toggle the whole badges system on or off depending on their specific needs.

The idea is that the badges subsystem is essentially working all the time, but notifications won't be triggered and the interface won't show up - the idea is that you can activate badges at any moment and scores will be right.

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG entry

📷 Screenshots (optional)

None

@ghost ghost assigned josepjaume Oct 8, 2018
@ghost ghost added the status: WIP label Oct 8, 2018

class AddBadgeSwitchToOrganizations < ActiveRecord::Migration[5.2]
def change
add_column :decidim_organizations, :badges_enabled, :boolean, null: false, default: false
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why default false?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't like true as a default for a boolean value in general, as it might be prone to errors as setting that value is not explicit. Which makes me think I forgot to set that value on organization creation!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this, to me it's kind of weird to have this as default false since by default we want it to be true.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can be explicit about it even if it's true at the database level.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'll break the tie: I'm with @josepjaume here

@ghost ghost added the status: WIP label Oct 8, 2018
@josepjaume josepjaume requested a review from oriolgual October 9, 2018 07:24
Copy link
Copy Markdown
Contributor

@mrcasals mrcasals left a comment

Choose a reason for hiding this comment

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

Nice job! Can you modify the decidim-core seeds so the default organization has the gamification system active, please? 😄

@ghost ghost added the status: WIP label Oct 9, 2018
@josepjaume
Copy link
Copy Markdown
Contributor Author

Nice catch @mrcasals! Done!

mrcasals
mrcasals previously approved these changes Oct 9, 2018
oriolgual
oriolgual previously approved these changes Oct 9, 2018
@josepjaume josepjaume dismissed stale reviews from oriolgual and mrcasals via 317d67d October 9, 2018 10:19
@ghost ghost added the status: WIP label Oct 9, 2018
@mrcasals mrcasals merged commit fa19822 into master Oct 9, 2018
@mrcasals mrcasals deleted the badges_toggle branch October 9, 2018 13:32
@josepjaume josepjaume added this to the CDP4 milestone Dec 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants