Skip to content

Feat/assign users on federated boards#7703

Open
grnd-alt wants to merge 2 commits intomainfrom
feat/assign-users-on-federated-boards
Open

Feat/assign users on federated boards#7703
grnd-alt wants to merge 2 commits intomainfrom
feat/assign-users-on-federated-boards

Conversation

@grnd-alt
Copy link
Copy Markdown
Member

@grnd-alt grnd-alt commented Mar 3, 2026

No description provided.

@grnd-alt grnd-alt force-pushed the feat/assign-users-on-federated-boards branch from 58cd339 to 275463a Compare March 3, 2026 10:46
@github-project-automation github-project-automation bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Productivity team Mar 3, 2026
@grnd-alt grnd-alt moved this from 🧭 Planning evaluation (don't pick) to 👀 In review in 📝 Productivity team Mar 3, 2026
@grnd-alt grnd-alt self-assigned this Mar 5, 2026
Signed-off-by: grnd-alt <git@belakkaf.net>
Signed-off-by: grnd-alt <git@belakkaf.net>
@grnd-alt grnd-alt force-pushed the feat/assign-users-on-federated-boards branch from 275463a to 30cad07 Compare March 25, 2026 12:55
@github-actions
Copy link
Copy Markdown
Contributor

🐢 Performance warning.
It looks like the query count of the integration tests increased with this PR.
Database query count is now 94756 was 93102 (+1.77%)
Please check your code again. If you added a new test this can be expected and the base value in tests/integration/base-query-count.txt can be increased.

if ($owner instanceof FederatedUser) {
return new FederatedUser($this->cloudIdManager->getCloudId($user['uid'], $owner->getCloudId()->getRemote()));
}
return null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe throw an exception instead of return null? if the owner isnt federated here.


public function localizeRemoteUsers(array $users, Board $localBoard) {
$localizedUsers = [];
foreach ($users as $i => $user) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$i is not used here, was there a reason for keeping it?

// local user for us = remote user for remote
$userId = $this->cloudIdManager->getCloudId($userId, null)->getId();
$type = Assignment::TYPE_REMOTE;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if ($this->cloudIdManager->isValidCloudId($userId)) {
$cloudId = $this->cloudIdManager->resolveCloudId($userId);
// assignee's origin is the same as the board owner's origin: send as local user
if ($cloudId->getRemote() === $ownerCloudId->getRemote()) {
$userId = $cloudId->getUser();
$type = Assignment::TYPE_USER;
}
} else {
// local user for us = remote user for remote
$userId = $this->cloudIdManager->getCloudId($userId, null)->getId();
$type = Assignment::TYPE_REMOTE;
}

this code block is duplicated in both assignUserOnRemote and unAssignUserOnRemote. maybe extract it into a private helper like resolveUserIdForRemote() to avoid the duplication and make maintenance easier for later?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 👀 In review

Development

Successfully merging this pull request may close these issues.

2 participants