Page MenuHomePhabricator

[lib] Introduce `threadPermissions.EDIT_THREAD_AVATAR`
ClosedPublic

Authored by atul on Apr 5 2023, 7:26 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 11, 5:47 AM
Unknown Object (File)
Sun, Nov 10, 6:23 PM
Unknown Object (File)
Sun, Nov 10, 1:29 PM
Unknown Object (File)
Sun, Nov 10, 10:15 AM
Unknown Object (File)
Sun, Nov 10, 10:15 AM
Unknown Object (File)
Sun, Nov 10, 9:56 AM
Unknown Object (File)
Sun, Nov 10, 9:15 AM
Unknown Object (File)
Thu, Nov 7, 1:09 PM
Subscribers

Details

Summary

Based heavily on changes to D7127. Included EDIT_THREAD_AVATAR everywhere that we were including EDIT_THREAD_NAME (please let me know if that was an incorrect assumption).

Test Plan
  1. Start up keyserver and ensure that the migration ran:

48e631.png (1×3 px, 435 KB)

  1. Check roles table to see if expected changes were made:

b49b03.png (786×2 px, 301 KB)

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

atul published this revision for review.Apr 5 2023, 7:32 AM

I don't think we want to allow EDIT_THREAD_AVATAR for PERSONAL and PRIVATE threads. I know right now we can modify the username for PERSONAL and PRIVATE, but we should probably change that in a future diff

In D7313#217566, @ginsu wrote:

I don't think we want to allow EDIT_THREAD_AVATAR for PERSONAL and PRIVATE threads. I know right now we can modify the username for PERSONAL and PRIVATE, but we should probably change that in a future diff

Makes sense, I'll remove those.

Does it make sense to just remove EDIT_THREAD_NAME permissions for threadTypes.PERSONAL now, so we don't have to run another migration for that?

keyserver/src/creators/role-creator.js
262 ↗(On Diff #24665)

Should we just remove this now?

265 ↗(On Diff #24665)

As @ginsu suggested, let's remove this

This revision is now accepted and ready to land.Apr 5 2023, 12:25 PM

Does it make sense to just remove EDIT_THREAD_NAME permissions for threadTypes.PERSONAL now, so we don't have to run another migration for that?

makes sense to me

Does it make sense to just remove EDIT_THREAD_NAME permissions for threadTypes.PERSONAL now, so we don't have to run another migration for that?

Maybe, but wouldn't that mean those who've already changed the name of a PERSONAL/PRIVATE thread wouldn't be able to change it back?

rebase before addressing feedback

In D7313#217779, @atul wrote:

Does it make sense to just remove EDIT_THREAD_NAME permissions for threadTypes.PERSONAL now, so we don't have to run another migration for that?

Maybe, but wouldn't that mean those who've already changed the name of a PERSONAL/PRIVATE thread wouldn't be able to change it back?

Yeah, that's true. I checked the database and several folks have changed PERSONAL thread names. (PRIVATE thread name changes don't appear to be possible currently.)

I guess we could implement a migration to reset the names... I guess you can create a Linear task before landing instead of making the change...

remove EDIT_THREAD_AVATAR from PERSONAL

This revision was landed with ongoing or failed builds.Apr 5 2023, 1:01 PM
This revision was automatically updated to reflect the committed changes.