Skip to content

adding a Comparison scalar and surfacing it in definition block for S…#260

Merged
cBoss1029 merged 7 commits intocanaryfrom
cbosselmann/add-comparison-scalar
Jun 15, 2022
Merged

adding a Comparison scalar and surfacing it in definition block for S…#260
cBoss1029 merged 7 commits intocanaryfrom
cbosselmann/add-comparison-scalar

Conversation

@cBoss1029
Copy link
Copy Markdown
Contributor

Adding updates that allow for comparison operators gte, gt, lte, lt to accept either string, number, or object without throwing TS errors

Changes

  • Added a Comparison custom scalar to be used with comparison operators gte, gt, lte, lt
  • surfaced compare method in definition block using asNexusMethod

Checklist

  • Requires dependency update?
  • [x ] Generating a new app works

Fixes #xxx

@cBoss1029 cBoss1029 marked this pull request as ready for review May 20, 2022 15:56
Copy link
Copy Markdown
Contributor

@kgajera kgajera left a comment

Choose a reason for hiding this comment

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

Because we are using the new compare type in the StringFilter, I'm wondering if this change makes sense. What are the use cases where I would want to pass a number, date, etc. when using a filter for "strings":

export const StringFilter = inputObjectType({
  name: 'StringFilter',
  description: 'A way to filter string fields. Meant to pass to prisma where clause',
  definition(t) {
    t.string('contains');
    t.string('endsWith');
    t.string('equals');
    t.compare('gt');
    t.compare('gte');
    t.list.nonNull.string('in');
    t.compare('lt');
    t.compare('lte');
    t.list.nonNull.string('notIn');
    t.string('startsWith');
  },
});

Does it make more sense for us to define a filter for each of our types like below so we don't sacrifice type safety:

export const StringFilter = inputObjectType({
  name: 'StringFilter',
  description: 'A way to filter string fields. Meant to pass to prisma where clause',
  definition(t) {
    t.string('contains');
    t.string('endsWith');
    t.string('equals');
    t.string('gt');
    t.string('gte');
    t.list.nonNull.string('in');
    t.string('lt');
    t.string('lte');
    t.list.nonNull.string('notIn');
    t.string('startsWith');
  },
});

export const NumberFilter = inputObjectType({
  name: 'NumberFilter',
  description: 'A way to filter number fields. Meant to pass to prisma where clause',
  definition(t) {
    t.int('equals');
    t.int('gt');
    t.int('gte');
    t.list.nonNull.int('in');
    t.int('lt');
    t.int('lte');
    t.list.nonNull.int('notIn');
  },
});

export const DateFilter = inputObjectType({
  name: 'DateFilter',
  description: 'A way to filter date fields. Meant to pass to prisma where clause',
  definition(t) {
    t.date('equals');
    t.date('gt');
    t.date('gte');
    t.list.nonNull.date('in');
    t.date('lt');
    t.date('lte');
    t.list.nonNull.date('notIn');
  },
});

@mthomps4
Copy link
Copy Markdown
Collaborator

mthomps4 commented May 25, 2022

Curious about the thoughts for the comment above as well.

@cBoss1029
Copy link
Copy Markdown
Contributor Author

True. I think that approach makes more sense. I'll refactor.

@cBoss1029 cBoss1029 force-pushed the cbosselmann/add-comparison-scalar branch from 3c66557 to 6b89083 Compare June 10, 2022 17:25
@cBoss1029 cBoss1029 merged commit 05644d6 into canary Jun 15, 2022
@cBoss1029 cBoss1029 deleted the cbosselmann/add-comparison-scalar branch June 15, 2022 19:23
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.

3 participants