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

Added fixed values option for scope array #286

Merged
merged 1 commit into from
Oct 3, 2017

Conversation

smoyte
Copy link
Contributor

@smoyte smoyte commented Sep 29, 2017

Hi there. Thanks for all the awesome work on this classic gem.

I'm suggesting the ability to include fixed values in the scope array so that I can ignore soft deleted items (via acts_as_paranoid).

Someone had suggested using a string scope for this but I was encountering a bug with that when moving an object from one scope to another -- it wasn't resetting the position to 1, even though it is able to do that correctly with the identical scope but in array format.

Debugging that was going to be tough so I went for this instead, since it's nicer/more readable syntax anyway IMO. If you want me to file an issue for the bug I can do that also.

Hope you like this!

@smoyte
Copy link
Contributor Author

smoyte commented Sep 29, 2017

Oh, FYI, simply using deleted_at as a regular scope param doesn't work because on destroy, deleted_at gets set first before AAL tries to do its rank adjustments.

@brendon
Copy link
Owner

brendon commented Sep 29, 2017

Hi @hooverlunch, thank you for your contribution! :)

In the past I've done things like this for complex scopes: #158 (comment)

Though this solution looks good too. @swanandp, what do you think? I'm happy to merge it as it's quite useful and I don't think it breaks any of the current API.

@brendon brendon requested a review from swanandp September 29, 2017 20:08
Copy link
Contributor

@swanandp swanandp left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for contributing!

@brendon brendon merged commit e42afcd into brendon:master Oct 3, 2017
@brendon
Copy link
Owner

brendon commented Oct 3, 2017

Thanks @swanandp. Released as 0.9.9. Great work!

@smoyte
Copy link
Contributor Author

smoyte commented Oct 3, 2017

Yay! Thanks you all for the prompt response!

@joshuapinter
Copy link
Contributor

Great friggin' work, @hooverlunch! Really cleans up complex scopes. 👍

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