Conversation
| if (group.name !== 'operationsWorkingGroupBeta') { | ||
| return invalidMetadata(`The ${group.name} is incompatible with the remarked moderatePost`) | ||
| } | ||
| const member = await getMemberById(store, workerId, ['metadata', 'metadata.externalResources']) |
There was a problem hiding this comment.
No need for metadata.externalResources
| return invalidMetadata(`${actor} is not an HR account`) | ||
| } | ||
|
|
||
| const member = await getMemberById(store, workerId, ['metadata']) |
There was a problem hiding this comment.
workerId is the memberId of the HR worker not that of the validator.
const {memberId, validatorAccount} = const { postId, rationale } = metadata.verifyValidator
The memberId is string but I need u64 for the getMemberById function. What should I do?
There was a problem hiding this comment.
Oh I completely missed that.
You could use:
import { createType } from '@joystream/types'
createType('MemberId', Number(metadata.verifyValidator.memberId))But getMemberById just converts the id into a string so maybe just change the getMemberById id argument to: MemberId | string.
| import { generateParamsFromAccountId } from '../../fixtures/membership/utils' | ||
| import { MembershipMetadata } from '@joystream/metadata-protobuf' | ||
|
|
||
| export default async function updatingVerifyAccount({ api, query }: FlowProps): Promise<void> { |
There was a problem hiding this comment.
This is not the actually testing the validator profile verification. To test this I think you would need to create a fixture like RemarkModeratePostsFixture. But that would be a lot of effort. @zeeshanakram3 in your opinion is this test worth the effort or can we skip it here ?
| moderatePost: { | ||
| postId: Long.fromString(String(u.postId)), | ||
| rationale: u.rationale, | ||
| }, |
Yes, so I think the integration-test should be added for this action. Although, there isn't any specific place to add fixture for this specific action, but still I would say that add a new fixture After creating the new fixture, add a new flow (say Last step, add the flow ( cc. @chrlschwb |
thesan
left a comment
There was a problem hiding this comment.
Again the changes to these files are unnecessary:
metadata-protobuf/proto/Membership.protoquery-node/mappings/src/membership.tsquery-node/schemas/membership.graphqltests/network-tests/src/fixtures/membership/GiftMembershipHappyCaseFixture.tstests/network-tests/src/fixtures/membership/InviteMembersHappyCaseFixture.tstests/network-tests/src/fixtures/membership/UpdateProfileHappyCaseFixture.tstests/network-tests/src/fixtures/membership/utils.tstests/network-tests/src/flows/membership/updatingProfile.tstests/network-tests/src/graphql/queries/membership.graphql
They just make this PR more complex. Please remove them for now. You can add them back on #4915 later on. If after removing all these schema changes one these fixtures fails please just push the failing code and I'll have a look into it.
| protected assertQueryNodeEventIsValid(qEvent: MemberVerificationStatusUpdatedEventFieldsFragment, i: number): void { | ||
| console.log(qEvent, '---------------------------------------------------') | ||
|
|
||
| // assert.equal(qEvent.member.id, this.inputs[i % this.inputs.length].asMember.toString()) | ||
| // assert.equal(qEvent.account, this.inputs[i % this.inputs.length].account.toString()) | ||
| } |
There was a problem hiding this comment.
AFAIK this is not necessary:
| protected assertQueryNodeEventIsValid(qEvent: MemberVerificationStatusUpdatedEventFieldsFragment, i: number): void { | |
| console.log(qEvent, '---------------------------------------------------') | |
| // assert.equal(qEvent.member.id, this.inputs[i % this.inputs.length].asMember.toString()) | |
| // assert.equal(qEvent.account, this.inputs[i % this.inputs.length].account.toString()) | |
| } |
| }) | ||
| } | ||
|
|
||
| private assetVerifyValidatorTest(qMember: MembershipFieldsFragment[] | null): void { |
There was a problem hiding this comment.
Please rename this method to:
| private assetVerifyValidatorTest(qMember: MembershipFieldsFragment[] | null): void { | |
| private assertQueriedMembershipAreValid(qMember: MembershipFieldsFragment[] | null): void { |
| }) | ||
| } | ||
|
|
||
| private assetVerifyValidatorTest(qMember: MembershipFieldsFragment[] | null): void { |
There was a problem hiding this comment.
also please remove the commented code and console.log
| const verifyAccountFixture = new VerifyValidatorProfileFixture(api, query, VerifyValidator) | ||
| const remarkModerateRunner = new FixtureRunner(verifyAccountFixture) | ||
| await remarkModerateRunner.run() | ||
| await remarkModerateRunner.runQueryNodeChecks() |
| memberId: '1', | ||
| isVerified: true, | ||
| isVerifiedValidator: false, | ||
| }, | ||
| { | ||
| memberId: '2', | ||
| isVerified: true, | ||
| asWorker: 'j4VEC6FcJtBrwYQKhBAoB6Rj83jDeVua6azuHBrri1zoksBkz', | ||
| isVerifiedValidator: false, | ||
| }, | ||
| { | ||
| memberId: '3', | ||
| isVerified: false, | ||
| asWorker: 'j4VEC6FcJtBrwYQKhBAoB6Rj83jDeVua6azuHBrri1zoksBkz', | ||
| memberId: '27', | ||
| isVerifiedValidator: false, | ||
| }, |
There was a problem hiding this comment.
These 3 inputs are not actually changing the member metadata isVerifiedValidator. isVerifiedValidator should be set to true. If the tests don't pass it probably means that the mappings have a bug.
Also the 3 inputs are basically identical, so there's no point having them all. If you manage to add a membership WG worker and pass their id asWorker then 2 inputs should be enough: one without the asWorker field (so ran as a lead) and another with it. If not just one with { memberId: '1', isVerifiedValidator: false }.
| contentDirectoryJob | ||
| ) | ||
| job('updating member verify account', updateValidatorAccount).after(hireLeads) | ||
| job('updating member verify profile', updateValidatorAccount).after(hireLeads) |
| } | ||
|
|
||
| protected async getExtrinsics(): Promise<SubmittableExtrinsic<'promise'>[]> { | ||
| console.log(this.updates, '================+++++++++++++++++++=================') |
#4824