mirror of
https://github.com/opf/openproject.git
synced 2026-06-13 19:20:00 +00:00
Remove dead paginate entry point and clarify resolved-anchor recording
The Paginator.paginate class method bypassed the instance, discarding the resolved_anchor state the controller reads after .call; it had no callers, so drop it and keep the single new(...).call entry point. Extract the activity anchor side effect into record_resolved_anchor so the intent is explicit at the call site, and pin the server contract with a request spec asserting an unresolvable activity anchor omits the resolved-comment value.
This commit is contained in:
@@ -62,10 +62,6 @@ class WorkPackages::ActivitiesTab::Paginator
|
||||
include Pagy::Method
|
||||
include WorkPackages::ActivitiesTab::JournalSortingInquirable
|
||||
|
||||
def self.paginate(work_package, params = {})
|
||||
new(work_package, params).call
|
||||
end
|
||||
|
||||
attr_reader :work_package, :params, :filter, :resolved_anchor
|
||||
|
||||
def initialize(work_package, params = {})
|
||||
@@ -163,10 +159,7 @@ class WorkPackages::ActivitiesTab::Paginator
|
||||
activity_at, anchor_id = locate_anchor(anchor_type, target_record_id)
|
||||
return nil unless activity_at && anchor_id
|
||||
|
||||
# A comment anchor already matches the DOM's data-anchor-comment-id. An
|
||||
# activity anchor has no rendered counterpart, so hand the comment id it
|
||||
# resolved to back to the client to scroll to instead.
|
||||
@resolved_anchor = anchor_id if anchor_type.activity?
|
||||
record_resolved_anchor(anchor_type, anchor_id)
|
||||
|
||||
rows_ahead = scope
|
||||
.where("(journals.created_at, journals.id) > (?, ?)", activity_at, anchor_id)
|
||||
@@ -175,6 +168,13 @@ class WorkPackages::ActivitiesTab::Paginator
|
||||
(rows_ahead / limit) + 1
|
||||
end
|
||||
|
||||
# A comment anchor already matches the DOM's data-anchor-comment-id. An activity
|
||||
# anchor has no rendered counterpart, so hand the comment id it resolved to back
|
||||
# to the client to scroll to instead.
|
||||
def record_resolved_anchor(anchor_type, anchor_id)
|
||||
@resolved_anchor = anchor_id if anchor_type.activity?
|
||||
end
|
||||
|
||||
def locate_anchor(anchor_type, target_record_id)
|
||||
if anchor_type.comment?
|
||||
visible_journals.where(id: target_record_id).pick(:created_at, :id)
|
||||
@@ -190,9 +190,9 @@ class WorkPackages::ActivitiesTab::Paginator
|
||||
.pick(:created_at, :id)
|
||||
end
|
||||
|
||||
# Eager-loads and the sequence version are applied to the page slice here, not
|
||||
# to activities_scope, so the count query stays off them. Changeset journals
|
||||
# map back to Changeset records; work package journals go through the wrapper.
|
||||
# Eager-loads are applied to the page slice here, not to activities_scope, so
|
||||
# the count query stays off them. Changeset journals map back to Changeset
|
||||
# records; work package journals go through the wrapper.
|
||||
def load_activities(page_relation)
|
||||
journals = page_journals(page_relation)
|
||||
changesets = load_changesets(journals.select { changeset?(it) }.map(&:journable_id))
|
||||
|
||||
@@ -73,5 +73,12 @@ RSpec.describe "Work package activities tab",
|
||||
expect(response).to have_http_status(:ok)
|
||||
expect(response.body).not_to include(resolved_value_attribute)
|
||||
end
|
||||
|
||||
it "omits the resolved-comment value for an unresolvable activity anchor" do
|
||||
get work_package_activities_path(work_package), params: { anchor: "activity-999999" }
|
||||
|
||||
expect(response).to have_http_status(:ok)
|
||||
expect(response.body).not_to include(resolved_value_attribute)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user