Skip to content

refactor(room): Split two-arg setActiveSince() into single-arg setters#17811

Open
miaulalala wants to merge 4 commits intomainfrom
refactor/noid/room-setActiveSince-single-arg
Open

refactor(room): Split two-arg setActiveSince() into single-arg setters#17811
miaulalala wants to merge 4 commits intomainfrom
refactor/noid/room-setActiveSince-single-arg

Conversation

@miaulalala
Copy link
Copy Markdown
Contributor

@miaulalala miaulalala commented Apr 25, 2026

Part of the incremental migration of Room to extend OCP\AppFramework\Db\Entity (Phase 1d).

Room::setActiveSince(\DateTime $since, int $callFlag) combined two concerns — conditionally setting the active-since timestamp and bitwise-ORing the call flag — in a single two-argument method. Entity's magic __call() requires standard single-argument setters.

  • Replace with setActiveSince(?\DateTime $activeSince) and setCallFlag(int $callFlag)
  • Move the conditional/bitwise logic into RoomService::setActiveSince(), where it already lived conceptually

Why the split is safe

cc @nickvergessen

This differs from the related getLobbyState()/getLobbyTimer() refactor in an important way: there is no interdependency problem here.

The original setter's logic was already redundant:

  • The if (!$this->activeSince) guard in Room was defensive duplication — RoomService already ensures setActiveSince() is only called on the fresh-call path (when getActiveSince() === null), never on the upgrade-flags path.
  • The $this->callFlag |= $callFlag bitwise merge was also redundant — RoomService::setActiveSince() already computes $callFlag |= $oldCallFlag before calling Room's setter.

Events fire after both setters: On the fresh-call path, CallStartedEvent is dispatched after both room->setActiveSince() and room->setCallFlag(). No event handler observes state between the two calls. Same for the upgrade-flags path.

Getters were already pure: getActiveSince() and getCallFlag() never had side effects. The lobby refactor exposed a problem with lazy side effects in getters; that pattern does not exist here.

AI-Assisted-By: Claude Sonnet 4.6 noreply@anthropic.com

🛠️ API Checklist

🏁 Checklist

  • ⛑️ Tests (unit and/or integration) are included or not possible
  • 📘 API documentation in docs/ has been updated or is not required
  • 🔖 Capability is added or not needed

Part of the incremental migration of `Room` to extend
`OCP\AppFramework\Db\Entity` (Phase 1d).

`Room::setActiveSince(\DateTime $since, int $callFlag)` combined two
concerns — conditionally setting the active-since timestamp and
bitwise-ORing the call flag — in a single two-argument method.
Entity's magic `__call()` requires standard single-argument setters.

- Replace with `setActiveSince(?\DateTime $activeSince)` and `setCallFlag(int $callFlag)`
- Move the conditional/bitwise logic into `RoomService::setActiveSince()`,
  where it already lived conceptually

AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
…h call

Verifies that the bitwise OR previously in Room::setActiveSince() is
preserved after moving it to the service layer.

AI-Assisted-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Anna Larch <anna@nextcloud.com>
@miaulalala miaulalala added this to the 🏖️ Next Major (34) milestone Apr 25, 2026
@miaulalala miaulalala self-assigned this Apr 25, 2026
@miaulalala miaulalala added feature: api 🛠️ OCS API for conversations, chats and participants feature: conversations 👥 labels Apr 25, 2026
@miaulalala
Copy link
Copy Markdown
Contributor Author

@copilot resolve the conflicts

Co-authored-by: miaulalala <7427347+miaulalala@users.noreply.github.com>
Copy link
Copy Markdown

Copilot AI commented May 6, 2026

@copilot resolve the conflicts

Resolved in d67c7a2. Merged origin/main and fixed the conflict in tests/php/RoomTest.php by combining main's lobby-purity tests with this branch's setActiveSince/setCallFlag tests, and updating createRoom() to match main's updated signature (dropped ITimeFactory, added $lobbyTimer/$lobbyState params).

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

Labels

feature: api 🛠️ OCS API for conversations, chats and participants feature: conversations 👥

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants