-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Filter Feature #6684
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
Filter Feature #6684
Changes from 2 commits
e5a287a
d59ee6f
566714e
21c62f1
315bb97
cde07fd
9dc4fe9
8c72369
0bc6279
0e4b0dc
f16ad92
5c86ead
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import generateFilterQuery from "@reactioncommerce/api-utils/lib/generateFilterQuery.js"; | ||
|
|
||
| /** | ||
| * @name filterSearchAccounts | ||
| * @method | ||
| * @memberof GraphQL/Accounts | ||
| * @summary Query the Accounts collection for a list of customers/accounts | ||
| * @param {Object} context - an object containing the per-request state | ||
| * @param {Object} filter1level - an object containing ONE level of filters to apply | ||
| * @param {Object} filter2level - an object containing TWO levels of filters to apply | ||
| * @param {Object} filter3level - an object containing THREE levels of filters to apply | ||
| * @param {String} level - number of levels used in filter object | ||
| * @param {String} shopId - shopID to filter by | ||
| * @returns {Promise<Object>} Accounts object Promise | ||
| */ | ||
| export default async function filterSearchAccounts(context, filter1level, filter2level, filter3level, level, shopId) { | ||
| const { collections: { Accounts } } = context; | ||
|
|
||
| if (!shopId) { | ||
| throw new Error("shopId is required"); | ||
| } | ||
| await context.validatePermissions("reaction:legacy:accounts", "read", { shopId }); | ||
|
|
||
| const { filterQuery } = generateFilterQuery(context, "Account", filter1level, filter2level, filter3level, level, shopId); | ||
|
|
||
| return Accounts.find(filterQuery); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| import generateFilterQuery from "@reactioncommerce/api-utils/lib/generateFilterQuery.js"; | ||
|
|
||
| /** | ||
| * @name filterSearchCustomers | ||
| * @method | ||
| * @memberof GraphQL/Customers | ||
| * @summary Query the Accounts collection for a list of customers/accounts | ||
| * @param {Object} context - an object containing the per-request state | ||
| * @param {Object} filter1level - an object containing ONE level of filters to apply | ||
| * @param {Object} filter2level - an object containing TWO levels of filters to apply | ||
| * @param {Object} filter3level - an object containing THREE levels of filters to apply | ||
| * @param {String} level - number of levels used in filter object | ||
| * @param {String} shopId - shopID to filter by | ||
| * @returns {Promise<Object>} Accounts object Promise | ||
| */ | ||
| export default async function filterSearchCustomers(context, filter1level, filter2level, filter3level, level, shopId) { | ||
| const { collections: { Accounts } } = context; | ||
|
|
||
| if (!shopId) { | ||
| throw new Error("shopId is required"); | ||
| } | ||
| await context.validatePermissions("reaction:legacy:accounts", "read", { shopId }); | ||
|
|
||
| const { filterQuery } = generateFilterQuery(context, "Account", filter1level, filter2level, filter3level, level, shopId); | ||
|
|
||
| filterQuery.groups = { $in: [null, []] }; // filter out non-customer accounts | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we sure that this is a completely correct way to retrieve only customers?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The existing query endpoint uses the same approach. Hence I re-used it |
||
| return Accounts.find(filterQuery); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import getPaginatedResponse from "@reactioncommerce/api-utils/graphql/getPaginatedResponse.js"; | ||
| import wasFieldRequested from "@reactioncommerce/api-utils/graphql/wasFieldRequested.js"; | ||
|
|
||
| /** | ||
| * @name Query/accounts | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason this has to be a separate endpoint rather than functionality added on to the existing queries?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The existing query endpoint is using a fixed input structure like so The new filter has an entirely different input format to handle multiple conditions. So I assume it will break any existing implementation of the query if we change it.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we not extend it while not breaking the existing?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking that we keep the implementation of the new 'filter' feature as a standard/similar implementation across all the selected collections. Something like a common format including the name of the query endpoint. All the existing end-points (for accounts/customers/orders/products) accept input in different formats and we would have to make different changes in each of these to accept both the old & new format of input. Note: I have not checked, but we may again hit the earlier problem of graphql not flexible in allowing different input formats. Please let me know if we want to make these changes for all the collections or selected ones.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see the logic of having consistent naming of the endpoint across collections but I also think it's confusing to have two endpoints that do basically the same thing but differently. What do you suggest to resolve this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would you be okay to use 'filterXYZ' as the go-forward endpoint across all collections and mark the old ones as 'deprecated'? If so, we can add the new filter to all collections where there is something similar currently in place. Else, as you mentioned initially, we have to update the existing query end-points. We can start & test with a simple one like accounts (to confirm graphql does not create issues) and then implement that for the remaining 3 (customers/orders/products). In this case also, we may have to implement this update to other collections where there is something similar currently in place.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I'm fine with the "create new and deprecate" old approach
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I shall find other collections that are using 'filter' like query and add new filter along with the promotions-filter ticket
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, let's just stick with the three for now |
||
| * @method | ||
| * @memberof Accounts/Query | ||
| * @summary Query for a list of accounts | ||
| * @param {Object} _ - unused | ||
| * @param {Object} args - an object of all arguments that were sent by the client | ||
| * @param {String} args.shopId - id of shop to query | ||
| * @param {Object} args.filter1level - filter conditions with 1 level | ||
| * @param {Object} args.filter2level - filter conditions with 2 levels | ||
| * @param {Object} args.filter3level - filter conditions with 3 levels | ||
| * @param {String} args.level - filter level used | ||
| * @param {Object} context - an object containing the per-request state | ||
| * @param {Object} info Info about the GraphQL request | ||
| * @returns {Promise<Object>} Accounts | ||
| */ | ||
| export default async function filterSearchAccounts(_, args, context, info) { | ||
sujithvn marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| const { | ||
| shopId, | ||
| filter1level, | ||
| filter2level, | ||
| filter3level, | ||
| level, | ||
| ...connectionArgs | ||
| } = args; | ||
|
|
||
| const query = await context.queries.filterSearchAccounts(context, filter1level, filter2level, filter3level, level, shopId); | ||
|
|
||
| return getPaginatedResponse(query, connectionArgs, { | ||
| includeHasNextPage: wasFieldRequested("pageInfo.hasNextPage", info), | ||
| includeHasPreviousPage: wasFieldRequested("pageInfo.hasPreviousPage", info), | ||
| includeTotalCount: wasFieldRequested("totalCount", info) | ||
| }); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| import getPaginatedResponse from "@reactioncommerce/api-utils/graphql/getPaginatedResponse.js"; | ||
| import wasFieldRequested from "@reactioncommerce/api-utils/graphql/wasFieldRequested.js"; | ||
|
|
||
| /** | ||
| * @name Query/accounts | ||
| * @method | ||
| * @memberof Customers/Query | ||
| * @summary Query for a list of customers | ||
| * @param {Object} _ - unused | ||
| * @param {Object} args - an object of all arguments that were sent by the client | ||
| * @param {String} args.shopId - id of shop to query | ||
| * @param {Object} args.filter1level - filter conditions with 1 level | ||
| * @param {Object} args.filter2level - filter conditions with 2 levels | ||
| * @param {Object} args.filter3level - filter conditions with 3 levels | ||
| * @param {String} args.level - filter level used | ||
| * @param {Object} context - an object containing the per-request state | ||
| * @param {Object} info Info about the GraphQL request | ||
| * @returns {Promise<Object>} Accounts | ||
| */ | ||
| export default async function filterSearchCustomers(_, args, context, info) { | ||
| const { | ||
| shopId, | ||
| filter1level, | ||
| filter2level, | ||
| filter3level, | ||
| level, | ||
| ...connectionArgs | ||
| } = args; | ||
|
|
||
| const query = await context.queries.filterSearchCustomers(context, filter1level, filter2level, filter3level, level, shopId); | ||
|
|
||
| return getPaginatedResponse(query, connectionArgs, { | ||
| includeHasNextPage: wasFieldRequested("pageInfo.hasNextPage", info), | ||
| includeHasPreviousPage: wasFieldRequested("pageInfo.hasPreviousPage", info), | ||
| includeTotalCount: wasFieldRequested("totalCount", info) | ||
| }); | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.