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

Remove QA Content Feature #1924

Merged
merged 12 commits into from Feb 12, 2020
Merged

Remove QA Content Feature #1924

merged 12 commits into from Feb 12, 2020

Conversation

@huwd
Copy link
Contributor

huwd commented Feb 6, 2020

Trello: https://trello.com/c/qBM5QLID/482-business-finder-tech-debt

We are now in a position to wind down the Business finder tech debt.
This was implemented with a QA content feature which is no longer being used. This PR removes all this code, to simplify finder-frontend.

@huwd huwd changed the title Business checker tech debt 2 Remove QA Content Feature Feb 6, 2020
@huwd huwd changed the title Remove QA Content Feature [WIP] Remove QA Content Feature Feb 6, 2020
@koetsier

This comment has been minimized.

Copy link
Contributor

koetsier commented Feb 7, 2020

  • FacetExtractor can be removed / simplified
@bevanloon bevanloon temporarily deployed to finder-front-business-c-modkj9 Feb 10, 2020 Inactive
huwd added 2 commits Feb 6, 2020
class govuk-form-footer still seems to be being used in transition checker questions, as it's the last remaining css class, I've renamed the file to apply to brexit checker questions, and dumped everything else
@huwd huwd force-pushed the business-checker-tech-debt-2 branch from 5324f33 to a50b3b8 Feb 10, 2020
@bevanloon bevanloon temporarily deployed to finder-front-business-c-modkj9 Feb 10, 2020 Inactive
@huwd huwd mentioned this pull request Feb 10, 2020
@huwd huwd requested review from vanitabarrett, koetsier and laurentqro Feb 10, 2020
@huwd

This comment has been minimized.

Copy link
Contributor Author

huwd commented Feb 10, 2020

@koetsier so looks like that is still used:

Not really clear on how these work in other features... could we have a run through of what's going on here?

@vanitabarrett

This comment has been minimized.

Copy link
Contributor

vanitabarrett commented Feb 12, 2020

@huwd I wasn't aware https://www.gov.uk/uk-nationals-living-eu used the QA feature, but it looks like it does and is still live... so maybe we can't delete it quite yet. It's only one question though, looks like it only uses qa_to_content, so perhaps we can still remove the code specific to the business finder

@huwd huwd force-pushed the business-checker-tech-debt-2 branch from a50b3b8 to 3df8a33 Feb 12, 2020
@bevanloon bevanloon temporarily deployed to finder-front-business-c-modkj9 Feb 12, 2020 Inactive
@huwd

This comment has been minimized.

Copy link
Contributor Author

huwd commented Feb 12, 2020

@vanitabarrett I've removed the stuff that touched the QaToContentController.
The only bit I'm not sure about is this tracking, I'm not clear on if this should have also applied to the /uk-nationals-living-eu feature.

Would you be free to take a quick look over with me?

@huwd

This comment has been minimized.

Copy link
Contributor Author

huwd commented Feb 12, 2020

* FacetExtractor can be removed / simplified

@koetsier does it make sense to do that one in a separate PR?

@bevanloon bevanloon temporarily deployed to finder-front-business-c-modkj9 Feb 12, 2020 Inactive
@@ -6,5 +6,5 @@ $govuk-use-legacy-palette: false;
@import 'components/*';
@import 'finder_frontend';
@import 'views/search';
@import 'views/qa';
@import 'views/brexit_checker_question_page';

This comment has been minimized.

Copy link
@huwd

huwd Feb 12, 2020

Author Contributor

This is a rename - I removed all the qa related css, but there was one class that still seemed relevant to the brexit_checker, so i've renamed it and stuck it in it's own file below.

@@ -29,9 +29,6 @@
get "/*slug/email-signup" => "email_alert_subscriptions#new", as: :new_email_alert_subscriptions
post "/*slug/email-signup" => "email_alert_subscriptions#create", as: :email_alert_subscriptions

# Q&A frontend for "Find EU Exit guidance for your business" (www.gov.uk/find-eu-exit-guidance-business)
get "/prepare-business-uk-leaving-eu" => "qa#show"

# Q&A frontend for "UK Nationals in the EU" (www.gov.uk/uk-nationals-in-the-eu)
get "/uk-nationals-living-eu" => "qa_to_content#show"

This comment has been minimized.

Copy link
@huwd

huwd Feb 12, 2020

Author Contributor

Leaving this for now, it's still relevant.

@huwd huwd changed the title [WIP] Remove QA Content Feature Remove QA Content Feature Feb 12, 2020
@huwd huwd marked this pull request as ready for review Feb 12, 2020
@vanitabarrett

This comment has been minimized.

Copy link
Contributor

vanitabarrett commented Feb 12, 2020

@huwd I think you're right about the tracking, but I'm just double checking with Dilwoar as I've found two similar things: track-qa-choices and track-brexit-qa-choices and neither look to be used (apart from in the business finder)

Copy link
Contributor

koetsier left a comment

can remove:

-When /^I select choice "(.+)"/ do |label|
-  find("label", text: label).click
-end
-
-When /^I choose 'Yes' and select choice "(.+)"/ do |label|
-  choose "Yes", allow_label_click: true
-  find("label", text: label).click
-end
-
-When /^I skip the rest of the questions/ do
-  5.times do
-    click_on "Skip this question"
-  end
-end
-
-Then /^the correct facets have been pre-selected/ do
-  %w(aerospace products-or-goods).each do |value|
-    expect(find("input[value=#{value}]", visible: false).checked?).to eq(true)
-  end
-end

Copy link
Contributor

koetsier left a comment

Nice!

@huwd huwd merged commit 64a7db5 into master Feb 12, 2020
3 checks passed
3 checks passed
continuous-integration/jenkins/branch This commit looks good
Details
continuous-integration/jenkins/publishing-e2e-tests Publishing end-to-end tests succeeded on Jenkins
Details
continuous-integration/jenkins/security No security issues found
Details
@huwd huwd deleted the business-checker-tech-debt-2 branch Feb 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.