Skip to content

Commit

Permalink
Calculate value to be compared instead of using updated_at
Browse files Browse the repository at this point in the history
updated_at breaks when uploading attachments, as the same page will be
updated without us having a chance to update the header.

Instead, calculate an "etag" value from the meeting sections and agenda
items timestamps as well as the meeting's own lock_version.
  • Loading branch information
oliverguenther committed Jul 29, 2024
1 parent c4bcd97 commit 4c7d5c2
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export default class PollForChangesController extends ApplicationController {
updatedAt: String,
};

declare updatedAtValue:string;
declare referenceValue:string;
declare urlValue:string;
declare intervalValue:number;

Expand All @@ -58,7 +58,7 @@ export default class PollForChangesController extends ApplicationController {
}

async triggerTurboStream():Promise<void> {
await fetch(`${this.urlValue}?updatedAt=${this.updatedAtValue}`, {
await fetch(`${this.urlValue}?reference=${this.referenceValue}`, {
headers: {
Accept: 'text/vnd.turbo-stream.html',
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
render(Primer::OpenProject::PageHeader.new(
data: {
controller: "poll-for-changes",
poll_for_changes_updated_at_value: @meeting.updated_at.iso8601,
poll_for_changes_reference_value: @meeting.changed_hash,
poll_for_changes_url_value: check_for_updates_meeting_path(@meeting),
poll_for_changes_interval_value: check_for_updates_interval,
}
Expand Down
6 changes: 3 additions & 3 deletions modules/meeting/app/controllers/meetings_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ def show
end

def check_for_updates
if Time.zone.parse(params[:updatedAt]) < @meeting.updated_at.change(usec: 0)
respond_with_flash(Meetings::UpdateFlashComponent.new(meeting: @meeting))
else
if params[:updatedAt] == @meeting.changed_hash
head :no_content
else
respond_with_flash(Meetings::UpdateFlashComponent.new(meeting: @meeting))
end
end

Expand Down
19 changes: 19 additions & 0 deletions modules/meeting/app/models/meeting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,25 @@ class Meeting < ApplicationRecord
closed: 5
}

##
# Cache key for detecting changes to be shown to the user
def changed_hash
sql = <<~SQL

Check notice on line 116 in modules/meeting/app/models/meeting.rb

View workflow job for this annotation

GitHub Actions / rubocop

[rubocop] modules/meeting/app/models/meeting.rb#L116 <Rails/SquishedSQLHeredocs>

Use `<<~SQL.squish` instead of `<<~SQL`.
Raw output
modules/meeting/app/models/meeting.rb:116:11: C: Rails/SquishedSQLHeredocs: Use `<<~SQL.squish` instead of `<<~SQL`.
SELECT MAX(meeting_agenda_items.updated_at), MAX(meeting_sections.updated_at), MAX(meetings.lock_version) FROM meetings
LEFT JOIN meeting_agenda_items ON meeting_agenda_items.meeting_id = meetings.id
LEFT JOIN meeting_sections ON meeting_sections.meeting_id = meetings.id
WHERE meetings.id = :id
SQL

parts = ActiveRecord::Base
.connection
.exec_query(OpenProject::SqlSanitization.sanitize(sql, id:))
.rows
.flatten

OpenProject::Cache::CacheKey.expand(parts)
end

##
# Return the computed start_time when changed
def start_time
Expand Down

0 comments on commit 4c7d5c2

Please sign in to comment.