Skip to content

Conversation

oksanabaza
Copy link
Collaborator

@oksanabaza oksanabaza commented May 15, 2025

📝 Summary

Ticket Summary (Title):

Create View, Edit and Create pages to support the RBAC for VMs functionality

Ticket Link:

https://issues.redhat.com/browse/ACM-20531
https://issues.redhat.com/browse/ACM-20534
https://issues.redhat.com/browse/ACM-20718

Type of Change:

  • 🐞 Bug Fix
  • ✨ Feature
  • 🔧 Refactor
  • 💸 Tech Debt
  • 🧪 Test-related
  • 📄 Docs

✅ Checklist

General

  • PR title follows the convention (e.g. ACM-12340 Fix bug with...)
  • Code builds and runs locally without errors
  • No console logs, commented-out code, or unnecessary files
  • All commits are meaningful and well-labeled
  • All new display strings are externalized for localization (English only)
  • (Nice to have) JSDoc comments added for new functions and interfaces

If Feature

  • UI/UX reviewed (if applicable)
  • All acceptance criteria met
  • Unit test coverage added or updated
  • Relevant documentation or comments included

If Bugfix

  • Root cause and fix summary are documented in the ticket (for future reference / errata)
  • Fix tested thoroughly and resolves the issue
  • Test(s) added to prevent regression

🗒️ Notes for Reviewers

@oksanabaza oksanabaza changed the title Updated UI Create UI for RBAC for VMs - Edit and Create functionality May 15, 2025
@oksanabaza oksanabaza requested review from mshort55, Ginxo and kurwang May 15, 2025 15:49
@oksanabaza oksanabaza changed the title Create UI for RBAC for VMs - Edit and Create functionality Create UI for RBAC for VMs - View, Edit and Create functionality May 15, 2025
@oksanabaza oksanabaza changed the title Create UI for RBAC for VMs - View, Edit and Create functionality ACM-20531, ACM-20534, ACM-20718 - Create UI for RBAC for VMs - View, Edit and Create functionality May 15, 2025
…Edit and Create functionality

Signed-off-by: Oksana Bazylieva <obazylie@redhat.com>
Copy link
Collaborator

@kurwang kurwang left a comment

Choose a reason for hiding this comment

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

lgtm

</DescriptionListGroup>
)
}
case 'Radio':{
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to refactor to not have this duplicate code in lines 920-937 and 1268-1286?

Copy link
Collaborator

@mshort55 mshort55 left a comment

Choose a reason for hiding this comment

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

Looks good!

@mshort55 mshort55 merged commit 65283e3 into Ginxo:main May 15, 2025
}, [accessControl?.metadata])

useEffect(() => {
if (!isEditing && !isViewing && selectedUsers.length === 0) {
Copy link
Owner

Choose a reason for hiding this comment

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

> !selectedUsers.length ? 'true' : 'false'
'true'
> selectedUsers.length === 0 ? 'true' : 'false'
'true'

so 🤔

    if (!isEditing && !isViewing && !selectedUsers.length) {

const renderLabelGroup = () => {
return (
<LabelGroup isVertical numLabels={labels.length} expandedText={expandedText} collapsedText={collapsedText}>
<LabelGroup isVertical={props.isVertical ?? true} numLabels={labels.length} expandedText={expandedText} collapsedText={collapsedText}>
Copy link
Owner

Choose a reason for hiding this comment

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

right? 🤔

isVertical={props.isVertical}

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.

4 participants