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

WIP: store only one link check report #9989

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions app/components/admin/editions/tags_component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ def limited_access_tag
end

def broken_links_report_tag
return unless edition.link_check_reports.any? && edition.link_check_reports.last.completed?
return unless edition.link_check_report.present? && edition.link_check_report.completed?

return create_tag("Broken links") if edition.link_check_reports.last.broken_links.any?
return create_tag("Broken links") if edition.link_check_report.broken_links.any?

create_tag("Link warnings") if edition.link_check_reports.last.caution_links.any?
create_tag("Link warnings") if edition.link_check_report.caution_links.any?
end

def create_tag(label)
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/admin/editions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ def update
if updater.can_perform? && @edition.save_as(current_user)
updater.perform!

if @edition.link_check_reports.last
if @edition.link_check_report
LinkCheckerApiService.check_links(@edition, admin_link_checker_api_callback_url)
end

Expand Down Expand Up @@ -437,7 +437,7 @@ def edition_filter_options
.symbolize_keys
.merge(
include_unpublishing: true,
include_link_check_reports: true,
include_link_check_report: true,
include_last_author: true,
)
.merge(per_page: Admin::EditionFilter::GOVUK_DESIGN_SYSTEM_PER_PAGE)
Expand Down
6 changes: 3 additions & 3 deletions app/models/admin/edition_filter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def unpaginated_editions
editions = editions.review_overdue if review_overdue

editions = editions.includes(:unpublishing) if include_unpublishing?
editions = editions.includes(:link_check_reports) if include_link_check_reports?
editions = editions.includes(:link_check_report) if include_link_check_report?
editions = editions.includes(:last_author) if include_last_author?

@unpaginated_editions = editions
Expand Down Expand Up @@ -269,8 +269,8 @@ def include_unpublishing?
options.fetch(:include_unpublishing, false)
end

def include_link_check_reports?
options.fetch(:include_link_check_reports, false)
def include_link_check_report?
options.fetch(:include_link_check_report, false)
end

def include_last_author?
Expand Down
2 changes: 1 addition & 1 deletion app/models/edition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class Edition < ApplicationRecord
has_many :edition_authors, dependent: :destroy
has_many :authors, through: :edition_authors, source: :user
has_many :topical_event_featurings, inverse_of: :edition
has_many :link_check_reports, as: :link_reportable, class_name: "LinkCheckerApiReport"
has_one :link_check_report, as: :link_reportable, class_name: "LinkCheckerApiReport"

has_many :edition_dependencies, dependent: :destroy
has_many :depended_upon_contacts, through: :edition_dependencies, source: :dependable, source_type: "Contact"
Expand Down
4 changes: 2 additions & 2 deletions app/services/link_reporter_csv_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,9 @@ def timestamp(edition)
end

def broken_links(edition)
return [] if edition.link_check_reports.blank?
return [] if edition.link_check_report.nil?

edition.link_check_reports.last.broken_links.map(&:uri)
edition.link_check_report.broken_links.map(&:uri)
end

def edition_organisation(edition)
Expand Down
2 changes: 1 addition & 1 deletion app/sidekiq/check_organisation_links_worker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def find_organisation(organisation_id)
end

def public_editions(organisation)
least_recently_checked = Edition.includes(:link_check_reports).publicly_visible.with_translations.in_organisation(organisation).order("link_checker_api_reports.updated_at").limit(ORGANISATION_EDITION_LIMIT)
least_recently_checked = Edition.includes(:link_check_report).publicly_visible.with_translations.in_organisation(organisation).order("link_checker_api_reports.updated_at").limit(ORGANISATION_EDITION_LIMIT)
Edition.where(id: least_recently_checked.pluck(:id))
end
end
2 changes: 1 addition & 1 deletion app/views/admin/editions/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
content: simple_formatting_sidebar(
hide_inline_attachments_help: [email protected]_inline_attachments?,
show_attachments_tab_help: true,
link_check_report: LinkCheckerApiService.has_links?(@edition, convert_admin_links: false) ? @edition.link_check_reports.last : nil,
link_check_report: LinkCheckerApiService.has_links?(@edition, convert_admin_links: false) ? @edition.link_check_report : nil,
),
},
{
Expand Down
2 changes: 1 addition & 1 deletion app/views/admin/editions/show/_sidebar.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<%= render(
partial: "admin/link_check_reports/link_check_report",
locals: {
report: (@edition.link_check_reports.last || @edition.link_check_reports.build),
report: (@edition.link_check_report || @edition.link_check_report.build),
allow_new_report: true,
},
) if show_link_check_report?(@edition) %>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# To address https://github.com/alphagov/whitehall/issues/6011, we need
# to remove old link checker reports and switch to a 'has_one' relationship.

Edition.find_each do |edition|
reports = edition.link_check_reports.order(created_at: :desc).to_a
next if reports.size <= 1

# Keep the most recent report and delete the rest
reports_to_delete = reports.drop(1)
reports_to_delete.each(&:destroy)
end

Check failure on line 12 in db/data_migration/2025022610370000_remove_old_link_checker_reports.rb

View workflow job for this annotation

GitHub Actions / Lint Ruby / Run RuboCop

Layout/TrailingEmptyLines: 1 trailing blank lines detected. (https://rubystyle.guide#newline-eof)
Loading