Skip to content
New issue

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

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

Already on GitHub? # to your account

Add edition_id to link check reports #9986

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

ChrisBAshton
Copy link
Contributor

Adds edition_id association to Link Checker Reports. We'll run the migration and then remove the old link_reportable_type/link_reportable_id fields which are an unnecessary layer of abstraction.

The polymorphic association was added in November 2017 with no explanation: 03c4734

Since then, there doesn't seem to have been a single link check report which is associated with anything other than an Edition:

$ LinkCheckerApiReport.distinct.pluck(:link_reportable_type)
=> ["Edition"]

Therefore let's remove the obtuse reference and make it clear
that a link check report is associated with an edition (not a
'linkable'). This should also cut down on quite a lot of DB space,
since we're currently storing the word "Edition" on every single
link check report (almost 12 million rows in the DB).

Loosely related to https://trello.com/c/tmnht4P1/


⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

ChrisBAshton added a commit that referenced this pull request Feb 26, 2025
Following on from #9986 which introduces the 'edition_id' field,
we no longer need the old polymorphic fields.
@ChrisBAshton ChrisBAshton force-pushed the link-checker-report-edition-ids branch 2 times, most recently from f8e8858 to 6cf63dc Compare February 26, 2025 15:22
@ChrisBAshton ChrisBAshton changed the title Link checker report edition ids Add edition_id to link checker reports Feb 26, 2025
@ChrisBAshton ChrisBAshton changed the title Add edition_id to link checker reports Add edition_id to link check reports Feb 26, 2025
ChrisBAshton added a commit that referenced this pull request Feb 26, 2025
Following on from #9986 which introduces the 'edition_id' field,
we no longer need the old polymorphic fields.
@ChrisBAshton ChrisBAshton force-pushed the link-checker-report-edition-ids branch 3 times, most recently from 544636f to 3abd162 Compare February 28, 2025 10:29
@ChrisBAshton ChrisBAshton marked this pull request as ready for review February 28, 2025 10:29
Copy link
Contributor

@ryanb-gds ryanb-gds left a comment

Choose a reason for hiding this comment

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

Nice to simplify this a bit. We might need to have a chat to the platform team about how best to run long running migrations like this though

The polymorphic association was added in November 2017 with no
explanation: 03c4734

Since then, there doesn't seem to have been a single link check
report which is associated with anything other than an Edition:

```
$ LinkCheckerApiReport.distinct.pluck(:link_reportable_type)
=> ["Edition"]
```

Therefore let's remove the obtuse reference and make it clear
that a link check report is associated with an edition (not a
'linkable'). This should also cut down on quite a lot of DB space,
since we're currently storing the word "Edition" on every single
link check report (almost 12 million rows in the DB).

We'll deploy this to get the 'edition_id' in place, and start
setting it for new link check reports. In a subsequent PR we'll
add a data migration to copy over all 'link_reportable' IDs to
the new edition_id association, and then delete the obtuse
polymorphic reference.

Trello: https://trello.com/c/tmnht4P1
@ChrisBAshton ChrisBAshton force-pushed the link-checker-report-edition-ids branch from 3abd162 to 31e9ebd Compare March 3, 2025 15:44
In the next PR, we'll switch over to using this property and we'll
delete the `link_check_reports` property. We see no value in
retaining multiple link check reports per edition.

For now, we'll proceed with populating both `link_check_report`
and `link_check_reports`.
This will be run manually after the PR is merged.
@ChrisBAshton ChrisBAshton force-pushed the link-checker-report-edition-ids branch from d4656ea to 5d98840 Compare March 3, 2025 15:59
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants