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

Fix bug#109 #110

Merged
merged 20 commits into from
Sep 20, 2016
Merged

Fix bug#109 #110

merged 20 commits into from
Sep 20, 2016

Conversation

candyam5522
Copy link
Contributor

No description provided.

$('#passwordErrorMessage').text(xhr.responseText)
$('#passwordErrorMessage').text(xhr.responseText);
$("#passwordErrorMessage:contains('login')").html(function(_, html) {
return html.replace(/(login)/g, '<a href="/login">$1</a>');
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't want to use the responseText at all. The javascript should include its own message.

@candyam5522
Copy link
Contributor Author

Please review, thanks

$("#passwordErrorMessage:contains('login')").html(function(_, html) {
return html.replace(/(login)/g, '<a href="/login">$1</a>');
});
$('#passwordErrorMessage').html("Account already exists! You may go ahead and <a href='protected/login'>login</a>.");
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to check the status code and only print your error message if CONFLICT was detected. Otherwise, you need to show a different message or messages.

@candyam5522
Copy link
Contributor Author

Please review it, thanks

@@ -137,7 +137,14 @@ window.eureka.registration = new function () {
},
error: function (xhr, err) {
$('#passwordChangeFailure').show();
$('#passwordErrorMessage').text(xhr.responseText)
if (xhr.status == "409") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please make this a switch statement. Also, status should be returning a numerical value not a string.

@candyam5522
Copy link
Contributor Author

She wrote the msg as :
"An internal error has occurred. Please contact the technical team for further assistance."
Do you want me to use the same msg?

@candyam5522
Copy link
Contributor Author

Please review, thanks

@@ -44,7 +44,7 @@

<template:insert template="/templates/eureka_main.jsp">
<template:content name="content">
<h3>Account Settings ${user.username}</h3>
<h3>Account Settings1111 ${user.username}</h3>
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has been fixed in the later commit

@arpost
Copy link
Contributor

arpost commented Sep 19, 2016

Please resolve conflicts with pom.xml. Thx.

@candyam5522
Copy link
Contributor Author

I just solved the conflict in pom.xml file, you can merge now

@arpost arpost merged commit f22b97c into eurekaclinical:master Sep 20, 2016
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.

2 participants