Skip to content

[Minor fix] : Removed extra checks from isAdmin variable in Footer.jsx#6021

Closed
rautniraj wants to merge 3 commits intobigbluebutton:masterfrom
rautniraj:rautniraj-Footer.jsx
Closed

[Minor fix] : Removed extra checks from isAdmin variable in Footer.jsx#6021
rautniraj wants to merge 3 commits intobigbluebutton:masterfrom
rautniraj:rautniraj-Footer.jsx

Conversation

@rautniraj
Copy link
Copy Markdown
Contributor

@rautniraj rautniraj commented Mar 18, 2025

We dont need these two checks : currentUser && currentUser.role
We can handle this in a good way : currentUser?.role?.name === 'Administrator'

We dont need these two checks : currentUser && currentUser.role 

We are already handling this very safe : currentUser?.role.name === 'Administrator';
@rautniraj
Copy link
Copy Markdown
Contributor Author

rautniraj commented Mar 18, 2025

What i observe, we should also let super admin to view version tag in footer. My suggestion

const isAdmin = currentUser?.role?.name === 'Administrator' || currentUser?.role?.name === 'SuperAdmin';

@sonarqubecloud
Copy link
Copy Markdown

@rautniraj rautniraj changed the title Removed extra checks from isAdmin variable in Footer.jsx [Minor fix] : Removed extra checks from isAdmin variable in Footer.jsx May 4, 2025
@farhatahmad
Copy link
Copy Markdown
Collaborator

What i observe, we should also let super admin to view version tag in footer. My suggestion
const isAdmin = currentUser?.role?.name === 'Administrator' || currentUser?.role?.name === 'SuperAdmin'

Agreed - can you make this change?

…min, while considering for admin privilege
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 8, 2025

@rautniraj
Copy link
Copy Markdown
Contributor Author

Its done now. In Footer.jsx + SharedAccess.jsx element i have modified isAdmin check to include SuperAdmin also. These were the two places where isAdmin was being checked.

Important

Please share your view on my very old PR #5951 . Suggested changes will improve the UI and application logic.

@farhatahmad
Copy link
Copy Markdown
Collaborator

Was having issues merging this, so I created a new PR

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.

2 participants