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

Hide "Sync your account with existing owner" with no authentication method configured #1626

Merged
merged 2 commits into from
Feb 14, 2024

Conversation

anatolii-yemets
Copy link
Contributor

This pull request fixes the issue where a user receives a 500 Internal Server Error for owner association requests when trying to "Sync your account with existing owner" with no authentication method configured. The functionality is now disabled if no authentication method is provided in the config.

authType: DISABLED

image

authType: other options

image

Fixes #1380

@anatolii-yemets anatolii-yemets requested review from a team as code owners February 14, 2024 17:01
Copy link

sonarcloud bot commented Feb 14, 2024

Copy link

sonarcloud bot commented Feb 14, 2024

Quality Gate Passed Quality Gate passed for 'odd-platform-api'

Issues
0 New issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

Test Results

  55 files    55 suites   3m 0s ⏱️
259 tests 259 ✔️ 0 💤 0
287 runs  287 ✔️ 0 💤 0

Results for commit e86d509.

Comment on lines +25 to +27
const isShowOwnerAssociation = Boolean(
appInfo?.authType && appInfo.authType !== 'DISABLED'
);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it's a good practice to use useMemo hook for that types of variables but it's not necessary in that particular case. And it seems we don't need data type coercion here

Suggested change
const isShowOwnerAssociation = Boolean(
appInfo?.authType && appInfo.authType !== 'DISABLED'
);
const isShowOwnerAssociation = useMemo(() =>
appInfo?.authType && appInfo.authType !== 'DISABLED',
[appInfo?.authType])

@anatolii-yemets anatolii-yemets merged commit e44f37e into main Feb 14, 2024
8 checks passed
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.

Owner association request with disabled authentication config
3 participants