Skip to content

Conversation

@utchoang
Copy link

@utchoang utchoang commented Jan 13, 2022

Description

Fixes: #5618

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)

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?

@utchoang utchoang changed the base branch from main to 4.16 January 13, 2022 02:19
@utchoang
Copy link
Author

@blueorangutan ui

@blueorangutan
Copy link

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

@blueorangutan
Copy link

UI build: ✔️
Live QA URL: http://qa.cloudstack.cloud:8080/client/pr/5855 (SL-JID-1006)

@DaanHoogland
Copy link
Contributor

@utchoang both scenarios from issue #5618 remain unchanged. I see how this logic would be needed, but it looks like it is not enough, the menu is not reloaded.

@utchoang
Copy link
Author

utchoang commented Jan 13, 2022

@sureshanaparti @DaanHoogland cc @shwstppr This PR is not ready for review. Related #5618 (comment)

@DaanHoogland DaanHoogland self-assigned this Jan 13, 2022
@DaanHoogland DaanHoogland linked an issue Jan 13, 2022 that may be closed by this pull request
Copy link
Contributor

@shwstppr shwstppr left a comment

Choose a reason for hiding this comment

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

Code LGTM. Have not tested

@shwstppr shwstppr self-requested a review January 13, 2022 11:40
@sureshanaparti
Copy link
Contributor

@sureshanaparti @DaanHoogland cc @shwstppr This PR is not ready for review. Related #5618 (comment)

ok @utchoang let us know once the PR is ready.

Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

lgtm (tested by reporter on the qa site)

@DaanHoogland
Copy link
Contributor

if you think it is ready @utchoang , we can merge (2lgtm+tests)

@utchoang utchoang marked this pull request as ready for review January 14, 2022 00:42
@utchoang
Copy link
Author

@DaanHoogland Yes, if you tested it and it works fine then it's ready for merge.

@sureshanaparti
Copy link
Contributor

@DaanHoogland Yes, if you tested it and it works fine then it's ready for merge.

@adrianopaterlini tested this, and updated details in the issue #5618

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

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

Switching between domains with SAML user

5 participants