Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: pollId equals 0 #3650

Merged
merged 1 commit into from
Aug 22, 2024
Merged

fix: pollId equals 0 #3650

merged 1 commit into from
Aug 22, 2024

Conversation

hamza221
Copy link
Contributor

fix #3649

@hamza221 hamza221 requested a review from dartcafe August 15, 2024 09:38
@hamza221 hamza221 self-assigned this Aug 15, 2024
@hamza221 hamza221 marked this pull request as draft August 15, 2024 09:50
@hamza221 hamza221 marked this pull request as ready for review August 15, 2024 10:01
Copy link
Collaborator

@dartcafe dartcafe left a comment

Choose a reason for hiding this comment

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

Your approach aborts the watch, but it should still run, if no poll is loaded.

Instead the watch at the server side should just avoid the the db request and continue watching other poll`s changes to keep the polls list up to date.

The watch is meant to watch changes in the current poll and the polls list.

@dartcafe
Copy link
Collaborator

dartcafe commented Aug 16, 2024

A better approach would be to simply skip searching for a poll, if the id is 0 here:

public function watchUpdates(int $pollId, ?int $offset = null): array {
$this->pollMapper->find($pollId)->request(Poll::PERMISSION_POLL_VIEW);
$start = time();

Copy link
Collaborator

@dartcafe dartcafe left a comment

Choose a reason for hiding this comment

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

Ay. This will also result in an interruption of the watcher. If no pollId (aka 0) is given, the watcher should just not search for a poll, but still keep watching for changes.

I meant skipping the line 44 in case of no pollId given.

The job for the watcher is to wait for updates of the polls table and then return the changes back to the client to enable the client for real time updates.

Additionally, if a pollId is given, it deep watches for changes of the particular poll also (Votes, Option, comments, etc.).

@AndyScherzinger AndyScherzinger added this to the 7.3.0 milestone Aug 20, 2024
Signed-off-by: Hamza Mahjoubi <[email protected]>
@hamza221 hamza221 merged commit fb36605 into master Aug 22, 2024
21 checks passed
@dartcafe dartcafe deleted the fix/poll-id-0 branch October 2, 2024 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PollId = 0 in watchUpdates /apps/polls/poll/{id}/watch request
3 participants