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

Fix bug which caused jobseekers to be taken to saved jobs page after … #7197

Closed
wants to merge 3 commits into from

Conversation

KyleMacPherson
Copy link
Collaborator

@KyleMacPherson KyleMacPherson commented Oct 23, 2024

Why does visiting /jobseekers/edit redirect the user to the saved jobs page after log in?

When the jobseeker is redirected from the registrations controller to the sessions controller and forced to log in redirected? returns false, this causes us to set store_return_location to (jobseeker_root_path, scope: :jobseeker) the jobseeker root path being the saved jobs page, hence the issue we are currently seeing.

def new
    if (attempted_path = params[:attempted_path])
      alert_text = t("jobseekers.forced_login.#{forced_login_resource(attempted_path)}_html",
                     account_creation_link: helpers.govuk_link_to(t("jobseekers.forced_login.create_account"), new_jobseeker_registration_url))
      flash.now[:alert] = alert_text
    elsif (login_failure = params[:login_failure])
      alert_text = t("devise.failure.#{login_failure}")
      trigger_jobseeker_sign_in_event(:failure, alert_text)
      flash.now[:alert] = alert_text
    end

    super do
      unless redirected?
        store_return_location(jobseeker_root_path, scope: :jobseeker)
        session[:after_sign_in] = true
      end
    end
  end

redirected? looks like this:

def redirected?
  params[:redirected] == "true"
end

the reason this works for other paths is because the other controllers inherit from jobseekers base controller. The jobseekers base controller includes Authenticated which calls the authenticate_scope! method before each action and if it fails goes to the warden controller #jobseeker_forced_login action. This calls

def forced_login(scope)
  if attempted_path
    store_return_location(attempted_path, scope: scope)
  end
  # this
  params_hash = {
    redirected: true,
  }
params_hash[:login_failure] = login_failure if login_failure
  params_hash[:attempted_path] = attempted_path if forced_login_requires_alert?

  redirect_to send(:"new_#{scope}_session_path", params_hash)
end

The forced_login method above sets the redirected_true param which means that redirected? is true and the sessions#new action behaves in the standard devise manner.

Why does the registrations controller not do this?

Because it inherits directly from the Devise::SessionsController and so does not have hit our jobseeker_forced_login method which sets params_hash = { redirected: true }

Why don't we change the registrations controller to inherit from the jobseekers base controller?

The registrations_controller relies on a number of devise methods currently and there will quite a few changes that need to be made.

The redirect fix needs to be ready for emails to be sent out tomorrow.

We will also be doing away with this part of the code completely in the next 2-3 weeks as we deploy the one login changes so I don't think it is worth a large refactor at this point in time.

Trello card URL

Changes in this PR:

  • Is there anything specific you want feedback on?

Screenshots of UI changes:

Before

After

Next steps:

  • Terraform deployment required?

  • New development configuration to be shared?

…being forced to log in when trying to go to the jobseekers/edit page
Copy link

github-actions bot commented Oct 23, 2024

Review app https://teaching-vacancies-review-pr-7197.test.teacherservices.cloud was successfully deleted

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant