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

[AMBARI-23897] Log Search UI: login with invalid password – no error message is displayed #1337

Merged
merged 2 commits into from May 23, 2018

Conversation

tobias-istvan
Copy link
Member

What changes were proposed in this pull request?

Added error message when the auth service trigger auth error. To do this correctly I changed the auth service slightly. Also I simplified the unit test of the login component.

How was this patch tested?

Tested manually and with unit tests (267/267 success)

Please review Ambari Contributing Guide before opening a pull request.

@aBabiichuk please review it. Thanks.

@asfgit
Copy link

asfgit commented May 21, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/2391/
Test PASSed.

@aBabiichuk aBabiichuk self-requested a review May 21, 2018 17:21
@aBabiichuk aBabiichuk self-assigned this May 21, 2018
this.setLoginInProgressAppState(true);
const response$ = this.httpClient.postFormData('login', {
username: username,
password: password
});
}).share();
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be better to add share() globally for all requests (request() method of HttpClientService). It used to be implemented in such a way some time ago. Any reason why this attempt doesn't fit? share() was removed in scope of AMBARI-23514

logout(): Observable<Response> {
const response$ = this.httpClient.get('logout');
logout(): Observable<Boolean> {
const response$ = this.httpClient.get('logout').share();
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment for line 84, the same here

@@ -16,7 +16,7 @@
-->

<div class="login-form well col-md-4 col-md-offset-4 col-sm-offset-4">
<div class="alert alert-danger" *ngIf="isLoginAlertDisplayed" [innerHTML]="'authorization.error' | translate"></div>
<div class="alert alert-danger" *ngIf="isLoginAlertDisplayed" [innerHTML]="errorMessage"></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to use innerHTML attribute since there are no more HTML tags in the message

@@ -219,6 +219,8 @@
"logIndexFilter.update.error": "Error at updating Log Index Filter for cluster <span class='cluster-name'>{{cluster}}</span>. {{message}}",

"login.title": "Login",
"login.error.title": "Login error",
Copy link
Contributor

Choose a reason for hiding this comment

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

"authorization.error" prop can be removed

@asfgit
Copy link

asfgit commented May 23, 2018

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/Ambari-Github-PullRequest-Builder/2422/
Test PASSed.

@aBabiichuk aBabiichuk merged commit abb5295 into apache:trunk May 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants