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.

Sign up for GitHub

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

Jump to bottom

feat(hpc-vmware-managed-vcd): add iam checks #13384

Open
wants to merge 1 commit into
base: feat/hpc-vcd-beta
Choose a base branch
from
Open

feat(hpc-vmware-managed-vcd): add iam checks #13384

wants to merge 1 commit into from

Conversation

chipp972
Copy link
Contributor

@chipp972 chipp972 commented Oct 1, 2024

ref: MANAGER-15226

Question Answer
Branch? develop
Bug fix? no
New feature? yes
Breaking change? no
Tickets MANAGER-15226
License BSD 3-Clause
  • Try to keep pull requests small so they can be easily reviewed.
  • Commits are signed-off
  • Only FR translations have been updated
  • Branch is up-to-date with target branch
  • Lint has passed locally
  • Standalone app was ran and tested locally
  • Ticket reference is mentioned in linked commits (internal only)
  • Breaking change is mentioned in relevant commits

Description

Related

@chipp972 chipp972 requested review from a team as code owners October 1, 2024 16:13
@chipp972 chipp972 requested review from kqesar, frenautvh, MaximeBajeux, oalkabouss and anooparveti and removed request for a team October 1, 2024 16:13
@github-actions github-actions bot added feature New feature has conflicts Has conflicts to resolve before merging labels Oct 1, 2024
tibs245
tibs245 reviewed Oct 2, 2024
View reviewed changes
...pps/hpc-vmware-managed-vcd/src/components/tiles/editable-tile-item/EditableTileItem.spec.tsx
<EditableTileItem label={label} onClickEdit={() => {}} />,
<EditableTileItem
urn="urn"
iamActions={[]}
Copy link
Contributor

Choose a reason for hiding this comment

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

It's required to give empty array ? We cannot just no define iamActions ?

tibs245
tibs245 reviewed Oct 2, 2024
View reviewed changes
...pps/hpc-vmware-managed-vcd/src/components/tiles/editable-tile-item/EditableTileItem.spec.tsx
urn="urn"
iamActions={[]}
label={label}
onClickEdit={() => {}}
Copy link
Contributor

Choose a reason for hiding this comment

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

It's required to give empty function ? We cannot just no defineonClickEdit ?

Copy link
Contributor

@pauldkn pauldkn Oct 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

onClickEdit is not an optional props on this component
(because this component is used to edit content, should not be used without an edit function)

tibs245
tibs245 reviewed Oct 2, 2024
View reviewed changes
...cd/src/components/tiles/organization-general-information-tile/DatacentresCount.component.tsx
Comment on lines +25 to +29
return isLoading ? (
<OsdsSkeleton size={ODS_SKELETON_SIZE.xs} />
) : (
<Description>{vDatacentres?.data?.length.toString()}</Description>
);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return isLoading ? (
<OsdsSkeleton size={ODS_SKELETON_SIZE.xs} />
) : (
<Description>{vDatacentres?.data?.length.toString()}</Description>
);
if (isLoading) return <OsdsSkeleton size={ODS_SKELETON_SIZE.xs} />
return <Description>{vDatacentres?.data?.length.toString()}</Description>

tibs245
tibs245 reviewed Oct 2, 2024
View reviewed changes
packages/manager/apps/hpc-vmware-managed-vcd/src/types/iam.interface.ts Outdated
Comment on lines 1 to 5
export type Iam = {
id: string;
urn: string;
displayName?: string;
};
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not a general Type ? It's not possible to export it on IAM module ? May be a suggestion for next developpement ?

rjamet-ovh
rjamet-ovh reviewed Oct 2, 2024
View reviewed changes
...pps/hpc-vmware-managed-vcd/src/pages/dashboard/datacentre/compute/DatacentreCompute.page.tsx Outdated
@@ -49,6 +54,9 @@ export default function ComputeListingPage() {
const { t } = useTranslation('hpc-vmware-managed-vcd/datacentres/compute');
const { t: tVdc } = useTranslation('hpc-vmware-managed-vcd/datacentres');
const navigate = useNavigate();
const { data: vcdOrganization, isLoading } = useManagedVcdOrganization({
Copy link
Contributor

Choose a reason for hiding this comment

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

error management is missing

rjamet-ovh
rjamet-ovh reviewed Oct 2, 2024
View reviewed changes
...pps/hpc-vmware-managed-vcd/src/pages/dashboard/datacentre/storage/DatacentreStorage.page.tsx Outdated
@@ -52,6 +57,9 @@ export default function StorageListingPage() {
'hpc-vmware-managed-vcd/datacentres/compute',
);
const navigate = useNavigate();
const { data: vcdOrganization, isLoading } = useManagedVcdOrganization({
Copy link
Contributor

Choose a reason for hiding this comment

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

error management is missing as well

pauldkn
pauldkn reviewed Oct 2, 2024
View reviewed changes
...ager/apps/hpc-vmware-managed-vcd/src/components/tiles/billing-tile/BillingTile.component.tsx
Comment on lines +63 to +67
value: isLoading ? (
<OsdsSkeleton />
) : (
<Description>{user?.email}</Description>
),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why should we display Skeleton when isLoadingBillingServices while we might have user's data ?

...ager/apps/hpc-vmware-managed-vcd/src/components/tiles/billing-tile/BillingTile.component.tsx
Comment on lines +84 to +115
value: isLoading ? (
<OsdsSkeleton />
) : (
<div className="flex-wrap">
<div className="flex items-center gap-x-2">
<OsdsLink disabled>
{t('managed_vcd_dashboard_password_renew')}
</OsdsLink>
<OsdsTooltip className="flex items-center">
<OsdsIcon
className="cursor-pointer"
name={ODS_ICON_NAME.HELP}
size={ODS_ICON_SIZE.xxs}
color={ODS_THEME_COLOR_INTENT.text}
/>
<OsdsTooltipContent
slot="tooltip-content"
className="break-normal"
>
{t('managed_vcd_dashboard_password_tooltip')}
</OsdsTooltipContent>
</OsdsTooltip>
</div>
<OsdsChip
inline
color={ODS_THEME_COLOR_INTENT.primary}
className="ml-3"
size={ODS_CHIP_SIZE.sm}
>
{t('managed_vcd_dashboard_coming_soon')}
</OsdsChip>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why should we display Skeleton when isLoadingBillingServices while we have no data loading here ?

feat(hpc-vmware-managed-vcd): add iam checks
2b3e8f6 
ref: MANAGER-15226

Signed-off-by: Nicolas Pierre-charles <[email protected]>
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented Oct 2, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
4.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pauldkn pauldkn pauldkn left review comments

@rjamet-ovh rjamet-ovh rjamet-ovh left review comments

@tibs245 tibs245 tibs245 approved these changes

@kqesar kqesar Awaiting requested review from kqesar kqesar is a code owner automatically assigned from ovh/su-digital-tools-core-stack

@frenautvh frenautvh Awaiting requested review from frenautvh frenautvh is a code owner automatically assigned from ovh/su-digital-tools-core-stack

@MaximeBajeux MaximeBajeux Awaiting requested review from MaximeBajeux MaximeBajeux is a code owner automatically assigned from ovh/team-manager-dev-angularjs

@oalkabouss oalkabouss Awaiting requested review from oalkabouss oalkabouss is a code owner automatically assigned from ovh/team-manager-dev-angularjs

@anooparveti anooparveti Awaiting requested review from anooparveti anooparveti is a code owner automatically assigned from ovh/team-manager-dev-angularjs

Assignees
No one assigned
Labels
feature New feature has conflicts Has conflicts to resolve before merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@chipp972 @tibs245 @pauldkn @rjamet-ovh