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.

Sign up for GitHub

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

Jump to bottom

Remove specialist topic code from specs #2147

Merged
merged 1 commit into from
May 21, 2024
Merged

Remove specialist topic code from specs #2147

merged 1 commit into from
May 21, 2024

Conversation

unoduetre
Copy link
Contributor

@unoduetre unoduetre commented May 7, 2024 •
edited by hannako

What

This prepares for #2136 and is related to alphagov/gds-api-adapters#1250

Why

This PR has been extracted from #2136 as requested by @hannako

Trello ticket

@unoduetre unoduetre mentioned this pull request May 7, 2024
Merged
@unoduetre unoduetre marked this pull request as ready for review May 7, 2024 11:23
@unoduetre unoduetre changed the title Update the name of provider state in Pact tests Update the name of a provider state in Pact tests May 7, 2024
@unoduetre unoduetre changed the title Update the name of a provider state in Pact tests Update the name of a provider state for Pact tests May 7, 2024
@unoduetre unoduetre force-pushed the 2422-remove-specialist-topic-code-from-email-alert-api-l-pact-helper branch from 97b7230 to 7a32e37 Compare May 8, 2024 09:46
@unoduetre unoduetre changed the title Update the name of a provider state for Pact tests Remove specialist topic code from specs May 8, 2024
@unoduetre unoduetre force-pushed the 2422-remove-specialist-topic-code-from-email-alert-api-l-pact-helper branch 4 times, most recently from c43d4b0 to 288186f Compare May 9, 2024 14:31
@unoduetre unoduetre requested a review from hannako May 9, 2024 14:34
hannako
hannako reviewed May 9, 2024
View reviewed changes
Copy link
Contributor

@hannako hannako left a comment

Choose a reason for hiding this comment

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

Thanks for reworking the specs Mat! Sorry one more thing. Have only commented in a couple of places, but probably applies in most cases where you've used taxon instead of taxon_tree

spec/features/daily_digest_spec.rb Outdated Show resolved Hide resolved
spec/features/weekly_digest_spec.rb Outdated Show resolved Hide resolved
spec/integration/browsing_subscriber_lists_spec.rb Outdated Show resolved Hide resolved
spec/features/daily_digest_spec.rb Outdated Show resolved Hide resolved
@unoduetre unoduetre force-pushed the 2422-remove-specialist-topic-code-from-email-alert-api-l-pact-helper branch from 288186f to b8bb846 Compare May 13, 2024 08:39
@unoduetre
Copy link
Contributor Author

@hannako

I've checked and ContentChange also uses taxon_tree. This is added by this code in email-alert-service. This simplified things a bit. If a test already uses taxon_tree, I've used document_collections instead, as those are also used by the database entries.

@unoduetre unoduetre force-pushed the 2422-remove-specialist-topic-code-from-email-alert-api-l-pact-helper branch 5 times, most recently from f7bb5f3 to d6af1c2 Compare May 13, 2024 10:13
@unoduetre unoduetre requested a review from hannako May 13, 2024 10:41
@unoduetre unoduetre force-pushed the 2422-remove-specialist-topic-code-from-email-alert-api-l-pact-helper branch 4 times, most recently from 2d7fb54 to 96e81c8 Compare May 16, 2024 10:20
@unoduetre unoduetre requested a review from hannako May 16, 2024 10:28
hannako
hannako approved these changes May 16, 2024
View reviewed changes
Copy link
Contributor

@hannako hannako left a comment

Choose a reason for hiding this comment

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

Thank Mat, and nice work on removing references to policies as well.

@unoduetre unoduetre requested a review from KludgeKML May 16, 2024 11:30
@unoduetre unoduetre force-pushed the 2422-remove-specialist-topic-code-from-email-alert-api-l-pact-helper branch 8 times, most recently from 5921af5 to 3683989 Compare May 17, 2024 15:42
@unoduetre unoduetre force-pushed the 2422-remove-specialist-topic-code-from-email-alert-api-l-pact-helper branch from 3683989 to 4bde247 Compare May 17, 2024 15:59
KludgeKML
KludgeKML approved these changes May 20, 2024
View reviewed changes
Copy link
Contributor

@KludgeKML KludgeKML left a comment

Choose a reason for hiding this comment

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

LGTM (assuming the pact tests get fixed)

@unoduetre unoduetre merged commit 8150818 into main May 21, 2024
8 of 9 checks passed
@unoduetre unoduetre deleted the 2422-remove-specialist-topic-code-from-email-alert-api-l-pact-helper branch May 21, 2024 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KludgeKML KludgeKML KludgeKML approved these changes

@hannako hannako hannako approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@unoduetre @KludgeKML @hannako