Skip to content

Qn map metadata for external resources for members#3862

Merged
mnaamani merged 8 commits intoJoystream:masterfrom
thesan:qn-membership-external-resources
Jul 29, 2022
Merged

Qn map metadata for external resources for members#3862
mnaamani merged 8 commits intoJoystream:masterfrom
thesan:qn-membership-external-resources

Conversation

@thesan
Copy link
Copy Markdown
Collaborator

@thesan thesan commented Jun 7, 2022

Closes #2813

@vercel
Copy link
Copy Markdown

vercel bot commented Jun 7, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Updated
pioneer-testnet ⬜️ Ignored (Inspect) Jun 24, 2022 at 1:38PM (UTC)

@thesan
Copy link
Copy Markdown
Collaborator Author

thesan commented Jun 16, 2022

@mnaamani @Lezek123 the updating member profile test fails with:

AssertionError: expected null to equal 'Updated name'
    at UpdateProfileHappyCaseFixture.assertProfileUpdateSuccesful (/home/theo/Projects/Joystream/joystream/tests/network-tests/src/fixtures/membership/UpdateProfileHappyCaseFixture.ts:49:12)
    at /home/theo/Projects/Joystream/joystream/tests/network-tests/src/fixtures/membership/UpdateProfileHappyCaseFixture.ts:118:25
    at QueryNodeApi.tryQueryWithTimeout (/home/theo/Projects/Joystream/joystream/tests/network-tests/src/QueryNodeApi.ts:388:9)
    at runMicrotasks (<anonymous>)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at async UpdateProfileHappyCaseFixture.runQueryNodeChecks (/home/theo/Projects/Joystream/joystream/tests/network-tests/src/fixtures/membership/UpdateProfileHappyCaseFixture.ts:116:5)
    at async FixtureRunner.runQueryNodeChecks (/home/theo/Projects/Joystream/joystream/tests/network-tests/src/Fixture.ts:198:5)
    at async FixtureRunner.runWithQueryNodeChecks (/home/theo/Projects/Joystream/joystream/tests/network-tests/src/Fixture.ts:203:5)
    at async updatingProfile (/home/theo/Projects/Joystream/joystream/tests/network-tests/src/flows/membership/updatingProfile.ts:46:5)
    at async /home/theo/Projects/Joystream/joystream/tests/network-tests/src/Job.ts:103:11 {
  showDiff: true,
  actual: null,
  expected: 'Updated name',
  operator: 'strictEqual'
}

But I can't figure out why, and I didn't change this part of query-node/mappings/src/membership.ts. Any idea ?

@thesan thesan marked this pull request as ready for review June 20, 2022 08:51
@thesan thesan requested a review from Lezek123 June 20, 2022 18:40
@dmtrjsg dmtrjsg requested review from zeeshanakram3 and removed request for Lezek123 June 21, 2022 10:13
@dmtrjsg dmtrjsg mentioned this pull request Jun 21, 2022
Open
kdembler
kdembler reviewed Jun 21, 2022
View reviewed changes
query-node/schemas/membership.graphql
about: String

"Social media handles, email address..."
externalResources: [MembershipExternalResource] @derivedFrom(field: "memberMetadata")
Copy link
Copy Markdown
Collaborator

@kdembler kdembler Jun 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I would suggest to use [MembershipExternalResource!]! or at least [MembershipExternalResource!] so we don't need to deal with possible null values in the array

Copy link
Copy Markdown
Collaborator Author

@thesan thesan Jun 24, 2022

Choose a reason for hiding this comment

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

Somehow: [MembershipExternalResource] @derivedFrom(field: "memberMetadata")
generates: [MembershipExternalResource!] in the generated/graphql-server/generated/schema.graphql
While [MembershipExternalResource!] -> [MembershipExternalResource!]! 😕.

Most of the schema returns null instead of empty arrays so I just picked [MembershipExternalResource!] over [MembershipExternalResource!]! to stay consistent

zeeshanakram3
zeeshanakram3 reviewed Jun 23, 2022
View reviewed changes
@dmtrjsg dmtrjsg mentioned this pull request Jun 24, 2022
Closed
@thesan @zeeshanakram3
Fix asMembershipExternalResource
e15fad4 
Co-authored-by: Zeeshan Akram <37098720+zeeshanakram3@users.noreply.github.com>
@thesan thesan force-pushed the qn-membership-external-resources branch from 5e35aac to e15fad4 Compare June 24, 2022 13:38
@thesan
Copy link
Copy Markdown
Collaborator Author

thesan commented Jun 24, 2022

@zeeshanakram3 your fix fully solved the tests thanks again 🙏

Could you give the PR a review again please ?

@thesan thesan requested a review from zeeshanakram3 June 24, 2022 14:25
@dmtrjsg
Copy link
Copy Markdown

dmtrjsg commented Jun 30, 2022

@zeeshanakram3 was this done for both members AND channels? Or shall I raise a separate one for QN team to support this for channels? TY

@zeeshanakram3
Copy link
Copy Markdown
Contributor

zeeshanakram3 commented Jun 30, 2022

@zeeshanakram3 was this done for both members AND channels? Or shall I raise a separate one for QN team to support this for channels? TY

@dmtrjsg right now the support only exists for members.

@dmtrjsg
Copy link
Copy Markdown

dmtrjsg commented Jun 30, 2022 •
edited
Loading

@zeeshanakram3 thanks for clarifying!

Found it and external resources for channels are covered here, already in the sprint backlog for right after carthage work is done :) :

@mnaamani mnaamani self-requested a review July 13, 2022 14:05
mnaamani
mnaamani requested changes Jul 13, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@mnaamani mnaamani left a comment

Choose a reason for hiding this comment

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

Can you bump the metadata-protobuf package version from 2.2.0 to 2.3.0

@mnaamani mnaamani merged commit 89f2a1a into Joystream:master Jul 29, 2022
@dmtrjsg dmtrjsg mentioned this pull request Jul 29, 2022
Closed
5 tasks
@thesan thesan deleted the qn-membership-external-resources branch August 16, 2022 12:30
@thesan thesan mentioned this pull request Aug 16, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kdembler kdembler kdembler left review comments

@mnaamani mnaamani mnaamani requested changes

@zeeshanakram3 zeeshanakram3 zeeshanakram3 approved these changes

Assignees

No one assigned

Labels

query-node

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Metadata for external resources for members

5 participants

@thesan @dmtrjsg @zeeshanakram3 @mnaamani @kdembler