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

Popular feature #577

Merged
merged 1 commit into from
Aug 29, 2014
Merged

Popular feature #577

merged 1 commit into from
Aug 29, 2014

Conversation

lolaodelola
Copy link
Contributor

Feature for finding popular tags.

@@ -14,6 +14,9 @@ class Tag < ::ActiveRecord::Base
validates_uniqueness_of :name, if: :validates_name_uniqueness?
validates_length_of :name, maximum: 255

### SCOPES:
scope :popular, lambda { |limit = 20| order("taggings_count desc").limit(limit) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Use ruby 1.9.3 syntax for lambdas.

@seuros seuros added the feature label Aug 20, 2014
@seuros
Copy link
Collaborator

seuros commented Aug 20, 2014

Thank you for your contribution @damzcodes .
I think the scope naming is application specific, what do you think about most_used , we could also add less_used.
The tests are failing , i will need them to pass before merging
Could you also update the changeslog/readme.

cc @bf4

@lolaodelola
Copy link
Contributor Author

Hi @seuros I've refractored the code now. Ran the tests on my machine and they all pass.

@@ -20,6 +20,8 @@ def validates_name_uniqueness?
end

### SCOPES:
scope :most_used, -> (limit = 20) { order("taggings_count desc").limit(limit) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

->(limit = 20)
RBX don't like the space :/

@lolaodelola
Copy link
Contributor Author

The white space has been removed, hopefully it passes now.

@seuros
Copy link
Collaborator

seuros commented Aug 20, 2014

Rails 3.2 tests are failling :/ , i think we have to rewrite the test.

@ProGM
Copy link
Contributor

ProGM commented Aug 28, 2014

This feature is very cool! What is missing before it can be added?
Can I help?

@seuros
Copy link
Collaborator

seuros commented Aug 28, 2014

Indeed ❤️ .
@damzcodes, can you squash your commit ? I will merge once it green and release a new version.

Feature added to find popular tags

Refractored using new lambda syntax, added 'least' method

Removing white space

Fixing test so that taggable counts are set in rails 3.2

Restoring space for readability

Included information on new features 'most_used' & 'least_used'

Editted to display properly
seuros added a commit that referenced this pull request Aug 29, 2014
@seuros seuros merged commit 41a5edb into mbleigh:master Aug 29, 2014
@seuros
Copy link
Collaborator

seuros commented Aug 29, 2014

Thank you @damzcodes 💚

@lolaodelola
Copy link
Contributor Author

You're most welcome.

tekniklr pushed a commit to tekniklr/acts-as-taggable-on that referenced this pull request Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants