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

Olympia - membership mappings and query node integration tests #2276

Merged
merged 27 commits into from
Jun 8, 2021

Conversation

Lezek123
Copy link
Contributor

@Lezek123 Lezek123 commented Mar 22, 2021

This PR introduces mappings for membership module and updated testing setup and integration tests, which allow testing those mappings against current Olympia runtime.

PR based on #2249 in order to include relevant @joystream/types updates

@Lezek123 Lezek123 marked this pull request as draft March 22, 2021 11:59
# Build graphql-server customizing DB name
DB_NAME=${PROCESSOR_DB_NAME} yarn codegen:server
# FIXME: Tmp add "declaration": true to query-node tsconfig
sed -i 's/"compilerOptions": {/"compilerOptions": {\n "declaration": true,/' ./generated/graphql-server/tsconfig.json
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
sed -i 's/"compilerOptions": {/"compilerOptions": {\n "declaration": true,/' ./generated/graphql-server/tsconfig.json
sed -i.bak 's/"compilerOptions": {/"compilerOptions": {\n "declaration": true,/' ./generated/graphql-server/tsconfig.json
rm ./generated/graphql-server/tsconfig.json.bak

Unfortunately, the `sed -i ` does not work on Mac 😞 The best we can do here is to use .bak file and remove it (optionally).

Copy link
Contributor

@metmirr metmirr Mar 26, 2021

Choose a reason for hiding this comment

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

This shouldn't be necessary anymore with the hydra-cli 2.0.1-beta.17 version.

@Lezek123
Copy link
Contributor Author

Lezek123 commented Apr 2, 2021

Based on issue #2303 I extended the mappings and implemented some changes:

  • Added all membership events to the schema based on a following template:
enum EventType {
  MembershipBought,
  # ...
}

type Event @entity {
  "{blockNumber}-{indexInBlock}"
  id: ID!

  "Hash of the extrinsic which caused the event to be emitted"
  inExtrinsic: String

  "Blocknumber of a block in which the event was emitted."
  inBlock: Int!

  "Index of event in block from which it was emitted."
  indexInBlock: Int!

  "Type of the event"
  type: EventType!
}

type MembershipBoughtEvent @entity {
  "Generic event data"
  event: Event!

  "New membership created."
  newMember: Membership!

  "New member root account in SS58 encoding."
  rootAccount: String!

  "New member controller in SS58 encoding."
  controllerAccount: String!

  "New member handle."
  handle: String!

  "New member metadata"
  metadata: MemberMetadata!

  "Referrer member."
  referrer: Membership
}

# ...
  • Replaced Block entity references with simple block number fields
  • Separated metadata fields from Membership into MemberMetadata entity
  • Changed MembershipSystem to MembershipSystemSnapshot (snapshot is updated in each block where at least one of the values is changed)
  • Updated referralCut type due to runtime change (it's now Int instead of BigInt and represents percentage)
  • Added DataObject schemas and changed the avatar type in MemberMetadata, both in GraphQL schema and protobuffs (this is currently not supported by the runtime so avatar field is currently not used)
  • Added isFoundingMember field to Membership (currently not used)
  • Updated the integration tests for full coverage

Note that is is expected that the test check fails because the runtime has been updated on olympia branch recently (on the PR branch itself the tests should run succesfully)

@Lezek123
Copy link
Contributor Author

Lezek123 commented Apr 3, 2021

I realized that perhaps a better way to handle different events would be to have EventData as an union of specific event data variants, ie.:

enum EventType {
  MembershipBought,
  # ...
}

type MembershipBoughtEventData @variant {
  "New membership created."
  newMember: Membership!

  "New member root account in SS58 encoding."
  rootAccount: String!

  "New member controller in SS58 encoding."
  controllerAccount: String!

  "New member handle."
  handle: String!

  "New member metadata"
  metadata: MemberMetadata!

  "Referrer member."
  referrer: Membership
}

union EventData = MembershipBoughtEventData | #...


type Event @entity {
  "{blockNumber}-{indexInBlock}"
  id: ID!

  "Hash of the extrinsic which caused the event to be emitted"
  inExtrinsic: String

  "Blocknumber of a block in which the event was emitted."
  inBlock: Int!

  "Index of event in block from which it was emitted."
  indexInBlock: Int!

  "Type of the event"
  type: EventType!

  "Event data (based on event type)"
  data: EventData
}

It would then be much easier to query, filter and display recent events regardless of their type using just one query.

The tradeoff is that in this case we wouldn't be able to enforce a specific event type for a relation, but it doesn't seem that bad compared to the potential benefits.

In order for this to work we would need to be able to specify relations inside variants, which is currently not supported by Hydra, but I believe this is beeing worked on atm (Joystream/hydra#207)

@bedeho
Copy link
Member

bedeho commented Apr 4, 2021

The main obstacle I see here is that this may not even be possible, as each union is unpacked as one table, and we would basically need on the order of one column per field per even type. I am not sure what the upper bound is and what the performance impact is on queries and mutations.

It would then be much easier to query, filter and display recent events regardless of their type using just one query.

I am not sure I see why, can you elaborate?

In order for this to work we would need to be able to specify relations inside variants, which is currently not supported by Hydra

Indeed, but do we need filtering & ordering on such relationships also? because that is not planned, not sure how feasible that is.

type: EventType!

I do not believe you really need this here now, because you can query on the type by just using the native Hydra union case filtering based on JSONS.

@Lezek123
Copy link
Contributor Author

Lezek123 commented Apr 12, 2021

It would then be much easier to query, filter and display recent events regardless of their type using just one query.

I am not sure I see why, can you elaborate?

I didn't realize that Hydra supports ... on queries when using interfaces (https://dzhelezov.gitbook.io/hydra/docs/schema-spec/interfaces) In that case interfaces would probably be enough to achieve what I had in mind.

@Lezek123 Lezek123 force-pushed the olympia-integration-tests branch from cd6e5b7 to b02ca25 Compare April 19, 2021 05:59
@mnaamani mnaamani merged commit b02ca25 into Joystream:olympia Jun 8, 2021
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.

5 participants