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

feat(portal login): handle navigation as per status #111

Conversation

nidhigarg-bmw
Copy link
Contributor

Description

Handle portal login navigation as per applicationStatus and applicationType

Why

Portal login navigation handling

Issue

eclipse-tractusx/portal-frontend#468

Checklist

Please delete options that are not relevant.

  • I have followed the contributing guidelines
  • I have performed a self-review of my own code
  • I have successfully tested my changes locally

@nidhigarg-bmw nidhigarg-bmw marked this pull request as draft February 7, 2024 09:30
Copy link
Contributor

@oyo oyo left a comment

Choose a reason for hiding this comment

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

small change for readability

}
if (([ApplicationStatus.SUBMITTED, ApplicationStatus.DECLINED, ApplicationStatus.APPROVED].indexOf(status) === -1) && Object.values(ApplicationStatus).includes(status))
(applicationType === ApplicationType.INTERNAL) ? history.push('/landing') : history.push('/?overlay=consent_osp')
else if ([ApplicationStatus.SUBMITTED, ApplicationStatus.APPROVED, ApplicationStatus.DECLINED].indexOf(status) !== -1) window.location.replace('/home')
Copy link
Contributor

Choose a reason for hiding this comment

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

Lines 276 and 278 contain a lot of duplication. Write
const isADS = [...].includes(status)
and then use that value true or false in those lines.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oyo I made some changes and make it less complicated. Please recheck once

@nidhigarg-bmw nidhigarg-bmw marked this pull request as ready for review February 8, 2024 12:22
@nidhigarg-bmw nidhigarg-bmw added the priority PR needs to prioritized at review label Feb 8, 2024
Copy link
Contributor

@oyo oyo left a comment

Choose a reason for hiding this comment

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

some quick changes

Copy link
Contributor

Choose a reason for hiding this comment

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

don't check in this file - because it contains env specific values

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh sorry, it was done my mistake

}
if ([ApplicationStatus.SUBMITTED, ApplicationStatus.CONFIRMED, ApplicationStatus.DECLINED].includes(status))
location.href = '/'
else if (Object.values(ApplicationStatus).includes(status) && !location.search.includes('overlay=consent_osp')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we don't need this here - because this is specific to the frontend app
&& !location.search.includes('overlay=consent_osp')

if (applicationType === ApplicationType.INTERNAL)
history.push('/landing')
else
location.href = '/'
Copy link
Contributor

Choose a reason for hiding this comment

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

here I was wrong with my comment earlier - we must specify the parameter because the frontend app checks for it:
location href = '/?overlay=consent_osp'

Copy link

sonarqubecloud bot commented Feb 8, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@nidhigarg-bmw nidhigarg-bmw requested a review from oyo February 8, 2024 13:28
@oyo oyo merged commit 7e6e60f into eclipse-tractusx:release/v1.6.0-RC4 Feb 8, 2024
6 checks passed
@oyo oyo deleted the feature/468/PortalLoginNavigation branch February 8, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority PR needs to prioritized at review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants