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

Empty and Delete bucket modals #1652

Merged

Conversation

weirdwiz
Copy link
Contributor

@weirdwiz weirdwiz commented Oct 24, 2024

@alfonsomthd
Copy link
Collaborator

@weirdwiz You need to update translations.

onClick: () =>
launcher(LazyDeleteBucketModal, {
isOpen: true,
extraProps: { bucketName, noobaaS3, launcher, refreshTokens },
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
extraProps: { bucketName, noobaaS3, launcher, refreshTokens },
extraProps: { bucketName, noobaaS3, launcher },

@@ -99,7 +124,6 @@ const BucketOverview: React.FC<{}> = () => {

const { bucketName } = useParams();
const [searchParams] = useSearchParams();

Copy link
Collaborator

Choose a reason for hiding this comment

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

nits: space was here to separate hooks call from normal variable...

@@ -151,7 +189,7 @@ const BucketOverview: React.FC<{}> = () => {
: []),
];

const actions = () => {
const actions = (noobaaS3: S3Commands) => () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

plz fix linter issues reported by GH actions...

@weirdwiz weirdwiz force-pushed the object_bucket_delete branch 2 times, most recently from b2409a7 to 7efa163 Compare November 14, 2024 04:54
Comment on lines 198 to 208
{error ||
(deleteError && (
<Alert
variant={AlertVariant.danger}
isInline
title={t('Error')}
className="pf-v5-u-mt-md"
>
{error?.message || deleteError?.message}
</Alert>
))}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
{error ||
(deleteError && (
<Alert
variant={AlertVariant.danger}
isInline
title={t('Error')}
className="pf-v5-u-mt-md"
>
{error?.message || deleteError?.message}
</Alert>
))}
{(error || deleteError) && (
<Alert
variant={AlertVariant.danger}
isInline
title={t('Error')}
className="pf-v5-u-mt-md"
>
{error?.message || deleteError?.message}
</Alert>
)}

@SanjalKatiyar
Copy link
Collaborator

/approve
/lgtm

- Add Delete Bucket modal for bucket removal
- Add Empty Bucket modal for clearing bucket contents
- Implement success/failure alerts for operations
- Handle S3 API responses

Signed-off-by: Divyansh Kamboj <[email protected]>
@SanjalKatiyar
Copy link
Collaborator

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Nov 14, 2024
Copy link
Contributor

openshift-ci bot commented Nov 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: SanjalKatiyar, weirdwiz

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@SanjalKatiyar
Copy link
Collaborator

/test odf-console-e2e-aws

@alfonsomthd
Copy link
Collaborator

/override ci/prow/odf-console-e2e-aws

Copy link
Contributor

openshift-ci bot commented Nov 14, 2024

@alfonsomthd: Overrode contexts on behalf of alfonsomthd: ci/prow/odf-console-e2e-aws

In response to this:

/override ci/prow/odf-console-e2e-aws

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@openshift-merge-bot openshift-merge-bot bot merged commit 54d935e into red-hat-storage:master Nov 14, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants