Remove subscribe to feed link for licence finder #3097
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Allows us to check for the licence finder beyond the QueryBuilder class. The ContentItem model also seems a more appropriate place to verfiy the exact finder content item..
The plan is to remove the feed links from all finders, unless we gain evidence that it is needed. Until this decision is made, we don't want to introduce the feature for the new licence finder. A new generalised method could be implented (not use `is_licence_transaction?`) if we wanted to do this for a subset of finders, vs all.
1pretz1
force-pushed
the
remove-subscribe-to-feed-link
branch
from
f42d4d0
to
c9fccd5
Compare
The `LiveSearch` module would not be initialised if $atomAutodiscoveryLink returns a falsey value. Since the subscribe to feed link is removed, this is now an optional argument in `application.js`. The `live_search.js` module is updated to check if `$atomAutodiscoveryLink` is set before trying to interact with it.
…ink is set to null Since the `$atomAutodiscoveryLink` is now optional, we still want to ensure that all of the existing functionality works as expected.
KludgeKML
reviewed
Jul 11, 2023
|
Query: Has this been captured as tech debt so we don't forget to remove it later? Or is the removal likely to involve working in the same space anyway? |
KludgeKML
approved these changes
Jul 11, 2023
|
When the decision on feed links is decided then the changes will take place in the same part of the codebase @maxgds. A tech debt card does sound like a good idea though so removing the feed link doesn't slip from our (or another teams) radar. We could either create a general tech debt card, or ensure that the team leading on the removal (Kuba's team?) has it tracked on their board? |
jon-kirwan
approved these changes
Jul 14, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Collaborated with @MartinJJones for this piece of work 🙌
The plan is to remove the feed links from all finders, unless we gain
evidence that it is needed. Until this decision is made, we don't want to
introduce the feature for the new licence finder.
We've also had to update the JS to keep the live search feature working,
as removing the link removed this functionality. We've now added JS tests
to ensure the feature works when the feed link isn't present.
Trello:
https://trello.com/c/9RFdV1KK/2094-remove-feed-link-from-only-the-new-licence-finder
Visual Changes
Follow these steps if you are doing a Rails upgrade.