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

Implement distinct field option for select #11

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

EugeneButusov
Copy link
Collaborator

No description provided.

lib/scope.js Outdated
@@ -44,6 +45,10 @@ class RestifizerScope {
return this.checkActionName(ACTIONS.SELECT, ACTIONS.SELECT_ONE, ACTIONS.COUNT);
}

isSelectDistinct() {
return this.checkActionName(ACTIONS.DISTINCT);
Copy link
Owner

Choose a reason for hiding this comment

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

add it to isSelect, please

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@vedi, I just wanted to separate select and distinct, because in one of our projects the same collection post-processing lead to crashes.

Copy link
Owner

Choose a reason for hiding this comment

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

It's a select by the meaning, and you named the method isSelectDistinct approving this. If you have an issue in your project after this change, it's better to update the code in your project.

Just in case, I mean to add DISTINCT checking to isSelect, and keep this method as well.

@EugeneButusov
Copy link
Collaborator Author

@vedi, I have a better idea, will close this one and create additional PR.

@EugeneButusov EugeneButusov deleted the add-distinct-parameter branch October 29, 2017 10:53
@EugeneButusov EugeneButusov restored the add-distinct-parameter branch October 29, 2017 11:41
@EugeneButusov EugeneButusov reopened this Feb 5, 2018
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.

2 participants