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

Training with disabilities view on courses #4358

Merged
merged 2 commits into from
Jul 11, 2024

Conversation

CatalinVoineag
Copy link
Contributor

@CatalinVoineag CatalinVoineag commented Jul 10, 2024

Context

The course show page on find and the preview page on publish contains
too much information that might be off-putting for the user.

This commit tries to fix this by putting this section into its own view
accessible from the course page

Changes proposed in this pull request

347fce8 This implements the view/controller/routes changes

91f3fa1 This is a bit of a refactor of the goto methods that tell the about_your_organisation form where to redirect

Description of 91f3fa1:
There are a few goto methods that tell the about your organisation
form where to redirect the user after update or when the user clicks the
back links.

Basically, both back links and update success methods can go to the same
path based on the goto param.

Because of this and the ever growing of these goto params I moved some
logic that calculates the redirection in the
about_your_organisation_form. This way it can be tested properly.

Guidance to review

Review the code and check if the training for disabilities page works

Checklist

  • Make sure all information from the Trello card is in here
  • Attach to Trello card
  • Rebased main
  • Cleaned commit history
  • Tested by running locally
  • Inform data insights team due to database changes
Peek.2024-07-11.09-49.mp4

@CatalinVoineag CatalinVoineag self-assigned this Jul 10, 2024
@CatalinVoineag CatalinVoineag force-pushed the cv/couse-training-with-dissabilities branch 3 times, most recently from 035bf16 to 91f3fa1 Compare July 10, 2024 15:46
@@ -8,5 +8,4 @@ def param_form_key
end

def goto_preview? = params.dig(param_form_key, :goto_preview) == 'true'
def goto_provider? = params.dig(param_form_key, :goto_provider) == 'true'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is no longer needed after the refactor 91f3fa1

@@ -14,8 +14,8 @@ def goto_provider_value(param_form_key:, params:)
params[:goto_provider] || params.dig(param_form_key, :goto_provider)
end

def goto_provider?(param_form_key:, params:)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No longer needed

@CatalinVoineag CatalinVoineag marked this pull request as ready for review July 10, 2024 16:11
@CatalinVoineag CatalinVoineag changed the title Cv/couse training with dissabilities Training with disabilities view on courses Jul 10, 2024
@CatalinVoineag
Copy link
Contributor Author

flash[:success] = I18n.t('success.published')
redirect_to(details_publish_provider_recruitment_cycle_path(provider.provider_code, provider.recruitment_cycle_year))
end
redirect_to @about_form.update_success_path
Copy link
Contributor

Choose a reason for hiding this comment

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

This is so much cleaner, nice one!

)
end
end
alias back_path update_success_path
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

<% else %>
<%= govuk_back_link_to(details_publish_provider_recruitment_cycle_path(@provider.provider_code, @provider.recruitment_cycle_year)) %>
<% end %>
<%= govuk_back_link_to(@about_form.back_path) %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@elceebee elceebee left a comment

Choose a reason for hiding this comment

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

Great work taking the time to refactor this goto behaviour. I've tested the back links and other functionality in the review app and it all looks good to me. 👍

@kelliedesigner
Copy link

Content change to new view

  • Update title of the new view to Training with disabilities and other needs at [PROVIDER NAME]
Example of new view page title
Screenshot 2024-07-11 at 11 41 43

Content changes to course page (not detailed in the original ticket)

  • Update the contents list to Training with disabilities
Contents list example
Screenshot 2024-07-11 at 11 36 40
  • Update the <h2> to Training with disabilities
  • Add a full stop after the sentence, outside of the <a>
Example of training with disabilities section
Screenshot 2024-07-11 at 11 37 57

@CatalinVoineag CatalinVoineag force-pushed the cv/couse-training-with-dissabilities branch from 91f3fa1 to 3aa46e4 Compare July 11, 2024 11:13
The course show page on find and the preview page on publish contains
too much information that might be off-putting for the user.

This commit tries to fix this by putting this section into its own view
accessible from the course page
There are a few `goto` methods that tell the about your organisation
form where to redirect the user after update or when the user clicks the
back links.

Basically, both back links and update success methods can go to the same
path based on the `goto` param.

Because of this and the ever growing of these `goto` params I moved some
logic that calculates the redirection in the
about_your_organisation_form. This way it can be tested properly.
@CatalinVoineag CatalinVoineag force-pushed the cv/couse-training-with-dissabilities branch from 3aa46e4 to d30f7bf Compare July 11, 2024 11:25
@kelliedesigner
Copy link

Looks good to me!

@CatalinVoineag CatalinVoineag merged commit 87ba27f into main Jul 11, 2024
19 checks passed
@CatalinVoineag CatalinVoineag deleted the cv/couse-training-with-dissabilities branch July 11, 2024 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants