[server] make first user owner #18417
Conversation
| if (userId) { | ||
| await this.auth.checkPermissionOnOrganization(userId, "write_members", orgId); | ||
| } | ||
| if (role !== "owner") { |
There was a problem hiding this comment.
If this is called a second time on the first user of an org, we would try to remove the owner role which would fail the whole operation. We instead now keep the first and only owner as an owner.
There was a problem hiding this comment.
Trying to understand the comment.
- This if-statement was checking the membership downgrade to guarantee there is at least a single owner left if the downgrade would be executed. ✔️
- Why would it be "called a second time on the first user of an org" ❓
There was a problem hiding this comment.
Why would it be "called a second time on the first user of an org"
That is because of the extra call in iam-session-app we added two days ago.
| "build": "yarn clean && yarn generate && yarn lint && npx tsc", | ||
| "lint": "yarn eslint src/*.ts src/**/*.ts", | ||
| "lint:fix": "yarn eslint src/*.ts src/**/*.ts --fix", | ||
| "build:clean": "yarn clean && yarn build", |
There was a problem hiding this comment.
not cleaning on yarn build doesn't make it faster but would leave *.js files in /dist when e.g. switching between branches.
| await os.addOrUpdateMember(owner.id, org.id, owner.id, "member"); | ||
| // verify they are still an owner | ||
| const members = await os.listMembers(owner.id, org.id); | ||
| expect(members.some((m) => m.userId === owner.id && m.role === "owner")).to.be.true; |
There was a problem hiding this comment.
we are not bailing when we try to make the last owner a member but just not do it.
svenefftinge
force-pushed
the
se/fix-signup
branch
from
7118e46 to
c251a69
Compare
|
@svenefftinge I can't get preview env work with command Server error logs, it seems you have activate it? |
svenefftinge
force-pushed
the
se/fix-signup
branch
2 times, most recently
from
2be9928 to
88c2944
Compare
| m.role === "owner", | ||
| ).length > 0; | ||
| if (!hasOtherRegularOwners) { | ||
| // first regular member is going to be an owner |
There was a problem hiding this comment.
The subtile change here is, that if the single owner would leave the org, the next sign-up would be promote to be an owner. before that, this role change would be only applied for the first regular member to join the org.
Need to think about this case first. (preview env is spinning up...)
There was a problem hiding this comment.
the single owner cannot leave an org and cannot be downgraded to a member. The change is that before we would throw an exception when someone tried to downgrade the single owner, where now we change the role to owner no matter what the previous state was. I agree the logic seems too involved, i.e.e the method does too much. Let me think how we can simplify things. maybe having two methods makes more sense. addMember and setRole.
svenefftinge
force-pushed
the
se/fix-signup
branch
from
88c2944 to
2992e54
Compare
|
/unhold |
Description
If this is called a second time on the first user of an org, we would try to remove the
ownerrole which would fail the whole operation. We instead now keep the first and only owner as an owner.Summary generated by Copilot
🤖 Generated by Copilot at 7118e46
Improved the robustness and functionality of the organization service and the monitoring endpoints in the server component. Fixed a potential issue with the build script and updated the corresponding test case.
Related Issue(s)
Fixes EXP-356
How to test
Documentation
Preview status
Gitpod was successfully deployed to your preview environment.
Build Options
Build
Run the build with werft instead of GHA
Run Leeway with
--dont-testPublish
Installer
Add desired feature flags to the end of the line above, space separated
Preview Environment / Integration Tests
If enabled this will build
install/previewIf enabled this will create the environment on GCE infra
Valid options are
all,workspace,webapp,ide,jetbrains,vscode,ssh. If enabled,with-previewandwith-large-vmwill be enabled./hold