This adds the dontCheckJoinPermissions option to createThread. commbot when creating the sidebar will require this as it will not be a member
Depends on D13813
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| keyserver/src/creators/thread-creator.js | ||
|---|---|---|
| 65 ↗ | (On Diff #45442) | This only applies for the changeRole for the viewer, not for the initial members / ghost members I think we should either give it a more specific name to clarify that, or make it apply for all of the changeRole calls |
| 367–369 ↗ | (On Diff #45442) | I might be misremembering, but I recall we had settled on having a flag that would avoid adding a commbot as a member at all, eg. we would skip this changeRole entirely Was there a reason that approach no longer made sense? |
| keyserver/src/updaters/thread-permission-updaters.js | ||
| 93 ↗ | (On Diff #45442) | This flag seems to do two things:
It would be good to at least add a code comment explaining that both of these things are affected |
| keyserver/src/creators/thread-creator.js | ||
|---|---|---|
| 367–369 ↗ | (On Diff #45442) | (I suppose we'd need to add commbot as a ghost at least) |
| keyserver/src/creators/thread-creator.js | ||
|---|---|---|
| 367–369 ↗ | (On Diff #45442) | I think we might have discussed having commbot as a ghost for the sidebar thread: // if the tagger is a user on Comm, then: // - call joinThread for them, not changeRole. we want the robotext // - construct a viewer with them // if the tagger is NOT a user on Comm, then: // - call changeRole/commitMembershipChangeset for commbot // - construct a viewer using commbot // we'll use the same viewer to create the sidebar later I was just working on a flag for not setting commbot to an admin. I think I approached it as setting commbot as a normal user first but I think setting it to a ghost user altogether makes sense to me. As you said, it removes the need to remove commbot |
| keyserver/src/creators/thread-creator.js | ||
|---|---|---|
| 367–369 ↗ | (On Diff #45442) | I wrote the above comment with a misunderstanding. I'm thinking back and now I think I understand. Didn't think to just skip the entire changeRole entirely and still thought we needed a dont check permission option. Will update to just skip the changeRole |