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

feat: hint banners in /status page #307 #418

Merged
merged 4 commits into from
Oct 17, 2024

Conversation

mickol34
Copy link
Collaborator

@mickol34 mickol34 commented Oct 8, 2024

Your checklist for this pull request

  • I've read the contributing guideline.
  • I've tested my changes by building and running mquery, and testing changed functionality (if applicable)
  • I've added automated tests for my change (if applicable, optional)
  • I've updated documentation to reflect my change (if applicable)

What is the current behaviour?

Currently user might not be aware of possible unwanted backend settings.

What is the new behaviour?

Now user receives warnings on /status page which warn them about missing agents, multiple agents connected with the same ursa_url or if it is advised to compact their datasets.

Test plan

Either backend setup should be messed up with or stub data filled into backend responses. Unfortunately I'm not currently aware of better methods without further messing with frontend code.

Closing issues

fixes #307

@mickol34
Copy link
Collaborator Author

mickol34 commented Oct 8, 2024

Screenshot from 2024-10-08 11-32-08

Here's how warnings look like if every warning is present. Notice how both ursa_url warning and datasets compacting warning are dismissable, but no agents warning is not.

@mickol34 mickol34 marked this pull request as ready for review October 8, 2024 10:21
@mickol34 mickol34 requested a review from msm-cert October 8, 2024 10:21
Copy link
Member

@msm-cert msm-cert left a comment

Choose a reason for hiding this comment

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

One small nitpick (the one about warning showing when refreshing the page is mostly what I'm asking about)

src/mqueryfront/src/status/StatusPage.js Show resolved Hide resolved
Comment on lines +41 to +43
${duplicateURLS.join(
", "
)}. Something might be wrong with backend configuration.`;
Copy link
Member

Choose a reason for hiding this comment

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

another nitpick, but maybe extract duplicateURLS.join to a separate variable (to avoid long expression inside)

Copy link
Member

Choose a reason for hiding this comment

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

I guess i'm merging it like it is.

Comment on lines +41 to +43
${duplicateURLS.join(
", "
)}. Something might be wrong with backend configuration.`;
Copy link
Member

Choose a reason for hiding this comment

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

I guess i'm merging it like it is.

@msm-cert msm-cert merged commit 8b8e12c into master Oct 17, 2024
10 checks passed
@msm-cert msm-cert deleted the feat/hint-banners-in-status-page-307 branch October 17, 2024 11:14
mickol34 added a commit that referenced this pull request Nov 6, 2024
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.

Implement some "hint" banners in the admin panel
2 participants