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

Fix problems with accounts queries #6060

Merged
merged 8 commits into from Jan 29, 2020

Conversation

aldeed
Copy link
Contributor

@aldeed aldeed commented Jan 28, 2020

Resolves #6006
Resolves #5906
Resolves #6017
Impact: minor
Type: feature|bugfix

Changes

  • Rather than accounts GQL query returning only non-admin accounts and requiring shopId param for a filter, it now returns all accounts and accepts an optional groupIds array for filtering. You can't filter by shopId because this no longer makes sense; accounts do not belong to a particular shop in 3.0.
  • The above change automatically fixed [3.0.0] Accounts query has duplicate field shopId  #6017 due to removing both shopId params
  • Account.groups was always null when requested through GraphQL. Now it's correct.
  • Removed any code where we were using account.shopId, which isn't usually set anymore
  • I also updated ESLint pkg version in order to get support for .cjs extension on .eslintrc file, and then changed that extension. This silences a warning that was showing when running lint command.

Breaking changes

The accounts GQL query takes different params and returns different results as described above.

Testing

Run GQL queries to verify the changes. I've updated the relevant integration tests, too.

Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
Filtering by shop ID is no longer possible because
accounts do not belong to any one shop.

Filtering by one or more account groups is now possible.

All accounts are returned, and not just non-admins.

Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
fixes #5906

Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
It's optional, we no longer set it, and
it doesn't mean anything

Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
to silent ES module warning

Signed-off-by: Eric Dobbertin <eric@dairystatedesigns.com>
@aldeed aldeed added this to In progress in 3.0.0 via automation Jan 28, 2020
@aldeed aldeed moved this from In progress to Review in progress in 3.0.0 Jan 28, 2020
Signed-off-by: Erik Kieckhafer <ek@ato.la>
Copy link
Member

@kieckhafer kieckhafer 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 overall.

Came across one issue that seems to maybe stem from the api-utils, not this PR. I've created a ticket for that issue, and also made a change in this PR to fix the issue for now.

3.0.0 automation moved this from Review in progress to Reviewer approved Jan 29, 2020
@aldeed aldeed merged commit c4b841b into release-3.0.0 Jan 29, 2020
3.0.0 automation moved this from Reviewer approved to Done Jan 29, 2020
@aldeed aldeed deleted the feat-aldeed-6006-accounts-query branch January 29, 2020 15:21
@kieckhafer kieckhafer mentioned this pull request Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
3.0.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants