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 graphql type generation after package upgrades #27738

Merged
merged 3 commits into from
Dec 23, 2018

Conversation

XavierM
Copy link
Contributor

@XavierM XavierM commented Dec 23, 2018

Summary

This fixes the SIEM UI graphql type generation after relevant packages have been upgraded in #25157.

Notes

  • Since the upstream package has split out the resolver type generation template, the client- and server-side types are now stored separately in public/graphql/types.ts and server/graphql/types.ts instead of a shared file in common/graphql. Client-side code should therefore exclusively use the client-side types while server-side code should only import the server-side types.
  • The introspection.json file has similarly been moved to the public/graphql directory.
  • The apollo resolver helper types have been adjusted to match the new Context type injection introduced by the upstream code generation package.

@elasticmachine
Copy link
Contributor

💔 Build Failed

@@ -77,7 +76,7 @@ export const formatUncommonProcessesData = (
fields.reduce(
(flattenedFields, fieldName) => {
flattenedFields.uncommonProcess._id = hit._id;
flattenedFields.uncommonProcess.instances = hit.total;
flattenedFields.uncommonProcess.instances = getOr(0, 'total.value', hit);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an interesting change. It is now hit.total.value rather than hit.total and it can be undefined as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

If so, that's fine, wondering if we can just directly use hit.total.value or if we have to use the getOr. I am fine with it if we have to use the getOr though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that was just an easy fix, I also want to investigate the relation attribute to make sure that we do the right thing here and for other query in the future

Copy link
Contributor

@FrankHassanabad FrankHassanabad left a comment

Choose a reason for hiding this comment

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

Good API upgrade change.

total: 1,
total: {
value: 1,
relation: 'eq',
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an interesting API output change.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@XavierM XavierM merged commit 9abded2 into elastic:feature-secops Dec 23, 2018
@XavierM XavierM deleted the graphql-types branch March 20, 2019 13:05
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