-
Notifications
You must be signed in to change notification settings - Fork 87
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
feat: Add community metadata validation #2073
Conversation
01ce34e
to
d2ec0fe
Compare
packages/backend/src/nest/storage/communityMetadata/communityMetadata.store.ts
Outdated
Show resolved
Hide resolved
packages/backend/src/nest/storage/communityMetadata/communityMetadata.store.ts
Outdated
Show resolved
Hide resolved
packages/backend/src/nest/storage/communityMetadata/communityMetadata.store.ts
Show resolved
Hide resolved
packages/state-manager/src/sagas/communities/communities.slice.ts
Outdated
Show resolved
Hide resolved
I'm still adding some tests, but I think this is ready for another look. Thanks! |
packages/backend/src/nest/storage/communityMetadata/communityMetadata.store.spec.ts
Outdated
Show resolved
Hide resolved
packages/state-manager/src/sagas/socket/startConnection/startConnection.saga.ts
Outdated
Show resolved
Hide resolved
c19e016
to
10f9bc8
Compare
Validate that the community metadata is only written to by the owner and refactor it into it's own store class.
10f9bc8
to
d1291fa
Compare
packages/backend/src/nest/storage/communityMetadata/communityMetadata.store.ts
Show resolved
Hide resolved
packages/backend/src/nest/storage/communityMetadata/communityMetadata.store.ts
Show resolved
Hide resolved
packages/backend/src/nest/storage/communityMetadata/communityMetadata.store.ts
Show resolved
Hide resolved
Please merge newest develop in Your branch and resolve conflicts in state manager. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd gone through all the changes and paid extra attention to the parts I'm confident with, although the size of this PR makes it hard to be fully accurate so there's a great chance I missed something. I think I'd be best if you also get approval from someone else.
I see there're still some conflicts and failing e2e tests but overall it looks fine.
packages/state-manager/src/sagas/messages/verifyMessage/verifyMessages.saga.ts
Outdated
Show resolved
Hide resolved
store.dispatch( | ||
communitiesActions.updateCommunityData({ | ||
id: community.id, | ||
// null/undefined type mismatch here. Might make things easier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's impossible as a factory for mocking data accepts certificate as null/undefined on purpose to perform some more advanced combinations, eg. mocking it automatically or storing and invalid redux entry basing on it's value. I think the best way is to just use // @ts-expect-error
knowing it's just a matter of mocking test data
@EmiM Do you think the e2e tests are failing because of changes in this PR? It also looks like a backend with tor test is failing. |
Yes but because of not compatible arguments after adding orbitdb key. Look at 'Guest clicks invitation link with invalid invitation code'. This test is skipped anyway so you can either add lacking argument or remove both 'Guest clicks invitation link with invalid invitation code' and 'Guest sees modal with warning about invalid code, closes it' Do backend tor tests pass on your local machine? If not then it may be because of this PRs changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be merged when e2e tests will pass
packages/backend/src/nest/storage/communityMetadata/communityMetadata.store.ts
Outdated
Show resolved
Hide resolved
When I run the tor tests locally, I get the following error:
|
Apparently, I am having trouble running Tor via those tests:
|
Pull Request Checklist
(Optional) Mobile checklist
Please ensure you completed the following checks if you did any changes to the mobile package: