-
Notifications
You must be signed in to change notification settings - Fork 14
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
community announcement via community factory contract #79
Conversation
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.
Nice! It feels it is 80% done
@frol PTAL. If it looks good, please approve and I'll do the manual migration, then merge PR. Otherwise I'll first address your concerns. The discussions part is awaiting requirements from Ori. Based on current requirement, my understanding is that's an indexer side thing (people add social db post with extra info to link it to an community, and frontend load these posts rendering in discussion), so won't require changes here. |
@elliotBraem I end up rename add_community_announcement to set_community_socialdb, the interface is as you imagined above. |
@ailisp I would just make it part of the UI implementation, so in order to start a discussion:
|
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.
@ailisp I have updated GitHub CI environment variable NEAR_GIGSBOARD_ACCOUNT_ID
to point to devhub.near
instead of devgovgigs.near
, so merging this PR won't change the production contract, instead, we can start working on slowly migrating everything over to the new contract.
let community_account_id: AccountId = | ||
format!("{}.{}", community, env::current_account_id()).parse().unwrap(); |
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.
This is also a good candidate for a dedicated method in near-account-id: near/near-account-id-rs#23
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.
Yeah this helper is also neat
@ailisp I've been setting up the develop branch for Announcements, it's configured with bodevhub.testnet. So far, I'm facing an issue where get_all_communities_metadata is giving an error: near contract call-function as-read-only bodevhub.testnet get_all_communities_metadata json-args {} network-config testnet now
Error:
0: Failed to fetch query for view method: 'get_all_communities_metadata' (contract <bodevhub.testnet> on network <testnet>)
1: Failed to make a view-function call
2: handler error: [Function call returned an error: wasm execution failed with error: HostError(GuestPanic { panic_msg: "Cannot deserialize element" })] You can see that issue here, as no communities are showing. If it is fixed and contract is upgraded, then this page should successfully show communities. Not sure if it's bad data or something with the PR Otherwise, creating community is working after attaching 2N and get_community has no issue: see here |
@elliotBraem I see what happened. There is a community created before a schema-breaking change. And I forgot to run migration before adding new communities. Let me fix it UPDATE: fixed. but i cleared the communities. You will need to recreate communities. |
Hi @elliotBraem I make some refactors to make the contract work on both mainnet & testnet. And redeploy a version to bodevhub.testnet & community.bodevhub.testnet. Please test again with frontend. For your convenience, here is my testing command for create community and add announcement:
|
It works on manual testing on testnet.
Basically this pr
TODO:
Note: doing migration and self-upgrade community factory similar to main contract should work, but I think it doesn't worth the effort: