We need this permission to create + update + remove farcaster channel tags
Linear task: https://linear.app/comm/issue/ENG-7648/introduce-a-new-permission-for-updatingmanaging-connecting-farcaster
Depends on D12400
Details
Details
Tested new clients:
Set the code version to current code version, created a new community and threads, confirmed that when an admin opened up the manage roles screen the manage farcaster channel tags permission was shown to the user and they could use it to create a new role.
Also was logging out the value of stateCheckStatus and confirmed that I always got
{ status: 'state_validated' }
Tested old clients + state sync mechanism:
Followed @inka's test plan in D12235 and confirmed that the value of stateCheckStatus was
{ status: 'state_validated' }
Also confirmed that when the user opened the community drawer that the manage farcaster channel tag permission was not shown for existing communities and newly created communities
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| lib/types/thread-permission-types.js | ||
|---|---|---|
| 272–273 ↗ | (On Diff #40264) | This copy needs to be reviewed |
| keyserver/src/database/migration-config.js | ||
|---|---|---|
| 826 ↗ | (On Diff #40264) | This is a very expensive operation, and can cause my keyserver to be down for 15-60 minutes. After you land this, I'll need to make sure that the new keyserver deploy I perform happens late at night. I think @inka is going to need to do something similar soon (see here) – I wonder if we could just run this migration once for both of those projects, instead of needing to run it twice. |
| lib/types/thread-permission-types.js | ||
| 272 ↗ | (On Diff #40264) | Farcaster needs to be capitalized... it's a proper noun... |
| native/redux/persist.js | ||
| 1293–1299 ↗ | (On Diff #40264) | Why don't we have an equivalent migration on web? In this case we probably need a real migration since we're gating on web codeVersion above. But even if we didn't, I think @tomek's recent changes require each native migration to have a corresponding web migration (and vice versa). (Not sure if these changes have been landed yet.) |
| native/redux/persist.js | ||
|---|---|---|
| 1293–1299 ↗ | (On Diff #40264) | My changes aren't landed yet - this will happen probably on Tuesday. I'll add a comment here if I land my changes before this diff is landed. (even if the corresponding web migration is added, we would need to maintain the same stare versions on both platforms) |
Comment Actions
Can we split this diff into two - one with migrations and one with the rest? The reason is we are supposed to run these migrations for both this code and my code in D12235. And I will need the diff with migrations to also change my FUTURE_CODE_VERSION to NEXT_CODE_VERSION flag.
| native/redux/persist.js | ||
|---|---|---|
| 1293–1299 ↗ | (On Diff #40264) | I landed diffs with the new approach to migrations. After rebasing, this migration should be placed in migrations instead of the legacyMigrations. Also, it should return all the DB ops instead of executing them. |
Comment Actions
I will be absent next 1.5 weeks, so remove me from review if needed. I just need this FUTURE_CODE_VERSION/NEXT_CODE_VERSION flag change
| lib/types/thread-permission-types.js | ||
|---|---|---|
| 283–284 | Let's go with this:
| |
| lib/types/thread-permission-types.js | ||
|---|---|---|
| 283–284 | All the other permission descriptions don't have a period at the end, so to keep this permission consistent with the rest, I dropped the period here too | |
Comment Actions
Thanks, my bad on that
| lib/types/thread-permission-types.js | ||
|---|---|---|
| 285 ↗ | (On Diff #41308) | Please keep lines to 80 chars |
| keyserver/src/fetchers/thread-fetchers.js | ||
|---|---|---|
| 297–300 | Thank you! | |