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

Gamification (badges) #3975

Merged
merged 1 commit into from
Aug 27, 2018
Merged

Gamification (badges) #3975

merged 1 commit into from
Aug 27, 2018

Conversation

josepjaume
Copy link
Contributor

@josepjaume josepjaume commented Aug 7, 2018

🎩 What? Why?

This feature adds user badges, which are earned when certain actions are performed.

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG entry
  • Add documentation regarding the feature
  • Add/modify seeds
  • Add tests

📷 Screenshots (optional)

None

private

def status
Decidim::Gamification.status_for(user, badge.name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Memoize this


delegate :current_user, to: :controller

def show
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this kind of show is not necessary

include Decidim::Core::Engine.routes.url_helpers
include Decidim::IconHelper

delegate :current_user, to: :controller, prefix: false
Copy link
Contributor

Choose a reason for hiding this comment

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

prefix isn't necessary

# inconsistent.
#
# Returns nothing.
def self.reset_badges(users = User.all)
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 prevent this at production?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's actually most useful in production, in case there's any data loss or inconsistency.

validates :levels, empty: false

validate do
errors.add(:levels, "level thresholds should be ordered") if levels.sort != levels
Copy link
Contributor

Choose a reason for hiding this comment

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

I18n?

Copy link
Contributor Author

@josepjaume josepjaume Aug 20, 2018

Choose a reason for hiding this comment

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

These messages aren't meant to reach user interfaces - it's an internal consistency validation. I don't think I18n make sense here.

@@ -24,6 +24,7 @@ def call

transaction do
@proposal.update published_at: Time.current
Decidim::Gamification.increment_score(@current_user, :proposals)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe increment the score for all the authors?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will look into that.

@ghost ghost added the status: WIP label Aug 20, 2018
@ghost ghost added the status: WIP label Aug 20, 2018
@ghost ghost added the status: WIP label Aug 20, 2018
@ghost ghost added the status: WIP label Aug 20, 2018
@josepjaume josepjaume merged commit 656d843 into master Aug 27, 2018
@josepjaume josepjaume deleted the feature/badges branch August 27, 2018 09:17
@josepjaume josepjaume added this to the CDP3 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.

2 participants