[lib] Introduce `threadPermissions.EDIT_THREAD_AVATAR`
ClosedPublic
Actions

Authored by atul on Apr 5 2023, 7:26 AM.

Details

Reviewers

ashoat
ginsu
• kuba

Commits

rCOMMf965bca79f04: [lib] Introduce `threadPermissions.EDIT_THREAD_AVATAR`

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

Start up keyserver and ensure that the migration ran:

Check roles table to see if expected changes were made:

Diff Detail

Repository

rCOMM Comm

Branch

arcpatch-D7313 (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

atul created this revision.Apr 5 2023, 7:26 AM

Herald added a subscriber: tomek. · View Herald TranscriptApr 5 2023, 7:26 AM

atul edited the test plan for this revision. (Show Details)Apr 5 2023, 7:29 AM

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.

Harbormaster completed remote builds in B18004: Diff 24665.Apr 5 2023, 7:43 AM

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

Harbormaster completed remote builds in B18022: Diff 24689.Apr 5 2023, 12:56 PM

https://linear.app/comm/issue/ENG-3607/remove-edit-thread-name-permission-forpersonal-threads

This revision was landed with ongoing or failed builds.Apr 5 2023, 1:01 PM

Closed by commit rCOMMf965bca79f04: [lib] Introduce `threadPermissions.EDIT_THREAD_AVATAR` (authored by atul). · Explain Why

This revision was automatically updated to reflect the committed changes.

atul added a commit: rCOMMf965bca79f04: [lib] Introduce `threadPermissions.EDIT_THREAD_AVATAR`.

Harbormaster completed remote builds in B18024: Diff 24691.Apr 5 2023, 1:05 PM