Skip to content

feat: add operator argument to GraphQlIdFilter #1522

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Mar 31, 2022
Merged

feat: add operator argument to GraphQlIdFilter #1522

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions projects/observability/src/shared/graphql/model/schema/filter/id/graphql-id-filter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,16 @@ import { GraphQlFilter, GraphQlFilterType, GraphQlOperatorType } from '../graphq
export class GraphQlIdFilter implements GraphQlFilter {
public readonly key: string = 'id';

public constructor(public readonly id: string, public readonly idScope: string) {}
public constructor(
public readonly id: string,
public readonly idScope: string,
public readonly operator: GraphQlOperatorType = GraphQlOperatorType.Equals
) {}

public asArgumentObjects(): [GraphQlIdFilterArgument] {
return [
{
operator: new GraphQlEnumArgument(GraphQlOperatorType.Equals),
operator: new GraphQlEnumArgument(this.operator),
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why we couldn't use a GraphQlFieldFilter if we want to use a different operator?

Copy link
Contributor Author

@jake-bassett jake-bassett Mar 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

We can, at least in the case I need this for. But using the ID filter is a little more semantically correct and helps readers better understand intent.

Not sure if there is more under the hood that this also helps, maybe @aaron-steinfeld can comment.

Copy link
Contributor

Choose a reason for hiding this comment

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

Besides the benefits to the reader Jake mentioned, the ID filter prevents hardcoding the keys and relationships between various scopes.

value: this.id,
type: new GraphQlEnumArgument(GraphQlFilterType.Id),
idType: new GraphQlEnumArgument(this.idScope)
Expand All @@ -21,7 +25,7 @@ export class GraphQlIdFilter implements GraphQlFilter {
// tslint:disable-next-line: interface-over-type-literal https://github.com/Microsoft/TypeScript/issues/15300
type GraphQlIdFilterArgument = {
value: GraphQlArgumentValue;
operator: GraphQlEnumArgument<GraphQlOperatorType.Equals>;
operator: GraphQlEnumArgument<GraphQlOperatorType>;
type: GraphQlEnumArgument<GraphQlFilterType.Id>;
idType: GraphQlEnumArgument<string>;
};