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

Add OperationForbiddenModal #3331

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add OperationForbiddenModal #3331

wants to merge 4 commits into from

Conversation

arbulu89
Copy link
Contributor

@arbulu89 arbulu89 commented Feb 25, 2025

Description

Add operation forbidden modal. We can render any kind of markdown and have specific description messages for each operation. For example:

image

PD: Don't check too much the text after the banner. We will review that in other PRs, when it is really implemented. This was just an example

How was this tested?

UT

@arbulu89 arbulu89 added the enhancement New feature or request label Feb 25, 2025
@arbulu89 arbulu89 marked this pull request as ready for review February 25, 2025 10:04
Copy link
Contributor

@abravosuse abravosuse left a comment

Choose a reason for hiding this comment

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

If we are going to review the banner text later on, then - code apart - this looks good to me.

@arbulu89
Copy link
Contributor Author

If we are going to review the banner text later on, then - code apart - this looks good to me.

Yes, the text that comes after the banner will depend on the operation itself.
The unique thing that are hardcoded now are the title and banner content (except the operation name itself, which comes as variable)

@abravosuse
Copy link
Contributor

Yes, the text that comes after the banner will depend on the operation itself. The unique thing that are hardcoded now are the title and banner content (except the operation name itself, which comes as variable)

No need to discuss it here but wondering if we are going to have a warning message tailored to the specific SAP workload(s) in the host or it's going to be a generic message trying to cover all possible cases. For example:

  • SAP workload managed by cluster :
    • ASCS/ERS workload
    • HANA workload
  • SAP workload not managed by cluster
    • ASCS/ERS workload
    • Application Server Instance workload
    • HANA workload

@arbulu89
Copy link
Contributor Author

@abravosuse I'm putting this PR back in draft. We clarified offline that we are going to do some adjustments to get the exact reasons of why the operation is forbidden to generate the modal message

@arbulu89 arbulu89 marked this pull request as draft February 25, 2025 13:07
@abravosuse
Copy link
Contributor

Perfect, @arbulu89 . Thank you!

@arbulu89
Copy link
Contributor Author

@abravosuse I have adjusted the component to accept a list of errors that we will send from the backend. This way we have a exact picture of why the operation was forbidden, so the user is aware of the reason.
For example (again, the texts are not implemented in this PR, we can change them).

  1. Cluster not in maintenance and hana running
    image
  2. Cluster in maintenance but hana running
    image
  3. Cluster not in maintenance and hana stopped
    image

All the texts are coming from the backend, who knows the exact reasons.
Now we can discuss about the styling @jagabomb hehe

@arbulu89 arbulu89 marked this pull request as ready for review February 25, 2025 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

2 participants