FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

dont show unauthorized notification on the login page by sudo87 · Pull Request #14054 · apache/cloudstack · GitHub

  1. apache
  2. cloudstack

Repository navigation

dont show unauthorized notification on the login page - #14054

Merged
harikrishna-patnala merged 2 commits into
apache:mainfrom
shapeblue:noSessionExpireNotification
Oct 8, 2026
ConversationCommits (2)ChecksFiles changed
Merged

dont show unauthorized notification on the login page#14054
harikrishna-patnala merged 2 commits into
apache:mainfrom
shapeblue:noSessionExpireNotification

Conversation

sudo87 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Description

This PR intends to fix #11721

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)
  • Build/CI
  • Test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

How did you try to break this feature and the system with this change?

sudo87 changed the title dont show unathorized notification for prelogin dont show unauthorized notification on the login page Sep 4, 2026

codecov Bot commented Sep 4, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 19.91%. Comparing base (ed1db53) to head (0187446).

Additional details and impacted files 
@@             Coverage Diff              @@
##               main   #14054      +/-   ##
============================================
- Coverage     19.91%   19.91%   -0.01%     
+ Complexity    20200    20196       -4     
============================================
  Files          6373     6373              
  Lines        577230   577234       +4     
  Branches      70696    70696              
============================================
- Hits         114958   114953       -5     
- Misses       449703   449715      +12     
+ Partials      12569    12566       -3
Flag Coverage Δ
uitests 3.70% <ø> (-0.01%) ⬇️
unittests 21.18% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sudo87 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

@blueorangutan ui

blueorangutan commented Sep 4, 2026

Copy link
Copy Markdown

@sudo87 a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress.

blueorangutan commented Sep 4, 2026

Copy link
Copy Markdown

UI build: ✔️
Live QA URL: https://qa.cloudstack.cloud/simulator/pr/14054 (QA-JID-986)

shwstppr approved these changes Sep 4, 2026
View reviewed changes

shwstppr left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
ui-login-notification.mp4

Looks good.
While it now shows the session notification once if you've a previously expired session, it doesn't show it every time like earlier

shwstppr requested a lite review from Copilot September 4, 2026 07:23
Copilot AI reviewed Sep 4, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

🟡 Changes recommended

The new requestCommand extraction misses the common POST case where config.data is a URLSearchParams instance, so allowlisted commands (e.g., forgotPassword) may still trigger the 401 flow unexpectedly.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes an unwanted “Unauthorized / session expired” notification being displayed on the login page by refining the UI’s global 401-response handling in the Axios error interceptor.

Changes:

  • Suppresses the generic 401 “unauthorized” notification when the current route is /user/login.
  • Attempts to better detect the failed API command for 401 responses (including parsing form-encoded POST bodies) so specific commands can bypass the global 401 flow.
  • Moves countNotify increment/commit to only occur when a notification is actually shown for 401 cases.
File summaries
File Description
ui/src/utils/request.js Refines global 401 handling to avoid showing the unauthorized notification on the login page and improves command detection for 401 exceptions.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread ui/src/utils/request.js Resolved
sudo87 requested a review from kiranchavala October 8, 2026 03:58
sudo87 marked this pull request as ready for review October 8, 2026 04:24
Copilot AI lite review requested due to automatic review settings October 8, 2026 04:28

sudo87 commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

thank you @shwstppr for testing the changes.

Copilot AI reviewed Oct 8, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

🔵 Needs a closer look

The unauthenticated listOauthProvider request is not covered by the notification suppression logic.

0 open findings

1 resolved since last review

🧠 Review effort: Lite

Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

harikrishna-patnala approved these changes Oct 8, 2026
View reviewed changes

harikrishna-patnala left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

code LGTM

harikrishna-patnala added this to the 24.0 milestone Oct 8, 2026
harikrishna-patnala merged commit 17294ca into apache:main Oct 8, 2026
23 of 27 checks passed
harikrishna-patnala deleted the noSessionExpireNotification branch October 8, 2026 10:53
Damans227 mentioned this pull request Oct 8, 2026
Merged
ribafish added a commit to ribafish/apache-cloudstack that referenced this pull request Oct 12, 2026
Merge remote-tracking branch 'origin/main' into gk/develocityIntegration
fc7b58a 
* origin/main: (156 commits)
  Upgrade Netris SDK version to 1.1.0 since 24.0 (apache#14365)
  feature: instance boot group and readiness rules (apache#13907)
  dont show unathorized notification for prelogin (apache#14054)
  .github: Upgrade docker/login-action to fix ASF Allowlist failure (apache#14276)
  virtual router: Add route-maps to BGP peers for Routed Mode (apache#9964)
  Add pre-commit hook to validate dependabot.yml (apache#14107)
  Add the ASF Allowlist check workflow (apache#14113)
  Revert "VMware to KVM: cold (VDDK) and warm (CBT) migration into Ceph/RBD and Linstor, plus importVm adoption of existing RBD root volumes and Linstor root/data volumes (apache#13656)"
  VMware to KVM: cold (VDDK) and warm (CBT) migration into Ceph/RBD and Linstor, plus importVm adoption of existing RBD root volumes and Linstor root/data volumes (apache#13656)
  engine-schema: upgrade path for 24.0.0 (apache#14033)
  xenserver/xcpng: do not bypass secondary storage when copy volumes between pools (apache#13750)
  backport: Incremental NAS backup support for KVM (apache#13074) (apache#13877)
  storage: KVM - enable RBD/Ceph volume encryption support (apache#13556)
  backup: remove the powered-off precondition for all backup providers (apache#14094)
  Nas backup: Fix mount/unmount error handling and timeout in LibvirtRestoreBackupCommandWrapper (apache#14006)
  kvm: apply rbd_default_data_pool when creating volumes from templates on RBD (apache#13361)
  server: copy template details to vm instance details when import VM (apache#12793)
  marvin: use pycryptodome instead of PyCrypt (apache#13594)
  Packaging: accept normalized Marvin archive names (apache#13793)
  SharedFS: fix NPE when accessing systemvm templates for unsupported hypervisors (apache#13930)
  ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
shwstppr shwstppr approved these changes
harikrishna-patnala harikrishna-patnala approved these changes
kiranchavala Awaiting requested review from kiranchavala

Assignees

No one assigned

Labels

component:UI

Projects

None yet

Milestone

24.0

Development

Successfully merging this pull request may close these issues.

5 participants

Back | FazBrowse Home | New Git URL