bug/WP-287: Remove login url redirect to profile data

[shayanaijaz]

Overview

Fixes the bug where in certain cases after logging in the user would be redirected to the /api/profile/data path and see a screen with api data text.

Related

• WP-287

Changes

• Removed an extra GET_PROFILE_DATA action that was being dispatched in AppRouter.jsx.
• Cleaned up tests for Allocations components by removing unused variables

Background

The profile data retrieved is added to the profile state variable. This state is only used in the ManageAccount components and ManageAccount.jsx already has a GET_PROFILE_DATA action being dispatched. So the data is available in that component and removing the action from the AppRouter does not have an effect here.

Previously, the GET_PROFILE_DATA was added in the AppRouter.jsx as part of this PR which was using the profile state to retrieve user information to display in Allocations tables (see AllocationsTable and AllocationsTeamViewModal).

In a later PR the usage of profile was removed from the Allocations components (see AllocationsTable and AllocationsTeamViewModal) However, that added GET_PROFILE_DATA action was not removed. Since the profile information is not being used in the Allocations components, the additional action being dispatched in AppRouter should be safe to remove

Testing

1. While logged out go to cep.test/public-data/
2. Log In and go through the flow and ensure page gets redirected to the My Account page

For reference, this was the flow before the fix:

Screen.Recording.2023-11-17.at.4.21.21.PM.mov

UI

Notes

[codecov]

Codecov Report

Merging #904 (cb97ae7) into main (1af7c8f) will decrease coverage by 0.01%.
The diff coverage is n/a.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #904      +/-   ##
==========================================
- Coverage   63.41%   63.41%   -0.01%     
==========================================
  Files         432      432              
  Lines       12393    12392       -1     
  Branches     2581     2581              
==========================================
- Hits         7859     7858       -1     
  Misses       4324     4324              
  Partials      210      210              

Flag	Coverage Δ
javascript	69.75% <ø> (-0.01%)	⬇️
unittests	56.94% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
client/src/components/Workbench/AppRouter.jsx	100.00% <ø> (ø)

[edmondsgarrett]

LGTM, working well!

[chandra-tacc]

Tested on dev.cep( the branch is deployed there right now).
No longer see the redirect on the specific repro.

I think we need more analysis on allowing only use of certain defined routes as part of 'next' parameter. How did a non-route-page like profile end up on next?I haven't checked the details, but will look into it now.

[rstijerina]

This fixes this specific issue 👍

I think a good long term solution to avoid similar cases would be to change this:

    redirect = getattr(settings, 'LOGIN_REDIRECT_URL', '/')
    if 'next' in request.session:
        redirect += '?next=' + request.session.pop('next')

    response = HttpResponseRedirect(redirect)
    return response

to this:

    redirect = getattr(settings, 'LOGIN_REDIRECT_URL', '/')

    response = HttpResponseRedirect(redirect)
    return response

[chandra-tacc]

This fixes this specific issue 👍

I think a good long term solution to avoid similar cases would be to change this:

    redirect = getattr(settings, 'LOGIN_REDIRECT_URL', '/')
    if 'next' in request.session:
        redirect += '?next=' + request.session.pop('next')

    response = HttpResponseRedirect(redirect)
    return response

to this:

    redirect = getattr(settings, 'LOGIN_REDIRECT_URL', '/')

    response = HttpResponseRedirect(redirect)
    return response

Tracking task: WP-395