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

transparent scope #67

Closed
wants to merge 3 commits into from
Closed

Conversation

gagalago
Copy link

@gagalago gagalago commented Dec 5, 2017

allow to chain scopes and scopes are transparent if no values are given to it.

fix #66

allow to chain scopes and scopes are transparent if no values are given to it.

fix state-machines#66
@seuros
Copy link
Member

seuros commented Dec 5, 2017

Thank you.
Can you add a test case please.

@gagalago gagalago force-pushed the patch-1 branch 6 times, most recently from 93f5134 to fad6f6a Compare December 28, 2017 15:43
end

# Generates the results for the given scope based on one or more states to filter by
def run_scope(scope, machine, klass, states)
Copy link
Author

@gagalago gagalago Dec 28, 2017

Choose a reason for hiding this comment

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

this is the same method as the one from state_machines https://github.com/state-machines/state_machines/blob/309668998449ca6c348de809f34660d822bc626e/lib/state_machines/machine.rb#L2130-L2135 but I just add compact.

I don't know how to change related gems in this pull request so I just override this method. Can you help me to do that?

gagalago added a commit to gagalago/state_machines that referenced this pull request Dec 29, 2017
this is needed for the pull request that allow transparent scope
state-machines/state_machines-activerecord#67
@gagalago
Copy link
Author

@seuros What do you think about this pull request?

@gagalago gagalago force-pushed the patch-1 branch 2 times, most recently from 59737a5 to ec6ea0e Compare February 28, 2018 14:44
end

# Generates the results for the given scope based on one or more states to filter by
def run_scope(scope, machine, klass, states)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this method added?

Copy link
Author

Choose a reason for hiding this comment

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

sorry, I add it to override this method from state machine (see state-machines/state_machines#63).
I don't know how to change gem dependency to do that in a good way. Can you give me some information, so that I can to it?

Copy link
Member

Choose a reason for hiding this comment

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

@diegotoral Are you up to rebase this PR ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not updated with the pull request but I can give it a try some day this week. 👍

Copy link
Member

Choose a reason for hiding this comment

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

Any update ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I have the branch rebased but I'm not allowed to push it here. 😬

Copy link
Member

Choose a reason for hiding this comment

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

Can you open a new pre ?

Copy link
Author

Choose a reason for hiding this comment

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

thanks to continuing my work 👍

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.

scope used on search application
4 participants