Add password confirmation to choose_password #3994

pungme · 2017-07-04T14:42:17Z

Would be great if the default update password template has the standard confirm password validation
Confirm password on Reset Password #1425
feel free to advice. Thanks! :-)

codecov · 2017-07-04T15:21:31Z

Codecov Report

Merging #3994 into master will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #3994      +/-   ##
==========================================
+ Coverage    92.2%   92.22%   +0.02%     
==========================================
  Files         116      116              
  Lines        8092     8092              
==========================================
+ Hits         7461     7463       +2     
+ Misses        631      629       -2

Impacted Files	Coverage Δ
...dapters/Storage/Postgres/PostgresStorageAdapter.js	`96.62% <0%> (+0.11%)`	⬆️
src/RestWrite.js	`93.34% <0%> (+0.18%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f2fc3ce...91f5670. Read the comment docs.

flovilmart

A few nits here and there, @natanrolnik can you validate the JS logic for me please?

flovilmart · 2017-07-08T18:15:24Z

views/choose_password

-
+
+    button:disabled,
+    button[disabled]{


nit space button[disabled] {

flovilmart · 2017-07-08T18:15:39Z

views/choose_password

@@ -126,6 +131,12 @@
      word-spacing: 0px; 
    }

+    #password_match_info{


nit space #password_match_info{

flovilmart · 2017-07-08T18:15:58Z

views/choose_password

@@ -156,12 +176,16 @@
        while (pair = tokenize.exec(querystring))
           urlParams[re_space(pair[1])] = re_space(pair[2]);
    })();
-
+    


remove white spaces :)

flovilmart · 2017-07-08T18:16:37Z

views/choose_password

    var id = urlParams['id'];
    var base = PARSE_SERVER_URL;
    document.getElementById('form').setAttribute('action', base + '/apps/' + id + '/request_password_reset');
    document.getElementById('username').value = urlParams['username'];
    document.getElementById('username_label').appendChild(document.createTextNode(urlParams['username']));
+    document.getElementById("password").oninput = validatePassword;


@natanrolnik not sure about that, should we use event listeners instead or is is valid?

flovilmart · 2017-07-08T18:16:57Z

views/choose_password

+    function validatePassword(){
+      var pass2 = document.getElementById("password").value;
+      var pass1 = document.getElementById("password_confirm").value;
+      if(pass1 !== pass2){


nit spaces if (pass1 !== pass2) {

flovilmart · 2017-07-08T18:17:01Z

views/choose_password

+      var pass2 = document.getElementById("password").value;
+      var pass1 = document.getElementById("password_confirm").value;
+      if(pass1 !== pass2){
+        if(document.getElementById("password_confirm").value){


flovilmart · 2017-07-08T18:17:04Z

views/choose_password

+          document.getElementById("change_password").disabled = true;
+          document.getElementById("password_match_info").innerHTML = "Must match the previous entry"
+        }
+      }else{


flovilmart · 2017-07-08T18:17:13Z

views/choose_password

+        document.getElementById("change_password").disabled = false;	 
+        document.getElementById("password_match_info").innerHTML = ""
+      }
+    //empty string means no validation error


nit alignment

flovilmart · 2017-07-08T18:17:53Z

@pungme nice feature, a few nits here and there but otherwise, I'll be happy to get that in!

pungme · 2017-07-09T09:41:38Z

Hey @flovilmart thanks for the review! I've changed base on your suggestion. Cheers

flovilmart · 2017-07-10T01:17:25Z

@pungme did you run some tests on that?

pungme · 2017-07-10T07:16:42Z

Hey @flovilmart yes i did ran the test on our dev environment. Or do you mean I should write some test on it?

pungme · 2017-07-13T06:44:48Z

Just tested on our production, it seems to work fine. let us know if there're anything we should change. Thanks!

…into pung-updateChoosePasswordTemplate

montymxb · 2017-09-10T01:42:02Z

@pungme this would be awesome if we can get the coverage up to pass. Can you go back over and add one or two tests to account for anything that's untested?

pungme · 2017-09-10T07:51:00Z

@montymxb sure! Sorry that i didn’t come back to this pr. I think i’ll have time again this week. :-)

montymxb · 2017-11-03T06:38:28Z

Whoop time flies! Thanks for getting the coverage up to spec @pungme .

@flovilmart JS logic looks good to me 👍
For your convenience here's what it looks like by default.

and here's what it looks like once you start typing in your password again, until it matches the existing entry. Just locks the button and displays the text, undoing it once they're both equivalent.

As far as setting an inline event callback, that will work just fine. The issue being you can't add more than one without overriding the existing entry for that event. If we want to add more than one listener per event we'll need something that accounts for IE < 9.

function addEventListener(elm, event, func) {
    if(elm.attachEvent) {
        // old IE
        return elm.attachEvent('on' + event, func);
    } else {
        // everything else
        return elm.addEventListener(event, func, false);
    }
}

// add to some element
addEventListener(document.getElementById('id'), 'input', function() {
    // do something on input...
});

flovilmart · 2017-11-03T11:26:52Z

Oh that’s amazing!

flovilmart · 2017-11-03T13:01:15Z

@montymxb you're afraid it won't fly on ie 9? Should we support it?

montymxb · 2017-11-03T20:06:38Z

Nope, should be just fine 👍 . It's just adding it like any other attribute of an html element. The only thing being if we ever wanted to go back to this and add another listener on the same event (on the same element) we would have to do the above method or something similar. Considering that I'm willing to give this an approval as is.

* update choose_password to have the confirmation * update commander * reverted the base * remove some spaces * nits * more nits

pungme added 4 commits July 4, 2017 16:27

update choose_password to have the confirmation

8598a5b

update commander

a93abd1

reverted the base

ae6e5de

remove some spaces

c67baab

pungme changed the title ~~update choose password html template~~ Add password confirmation to choose_password Jul 4, 2017

Merge branch 'master' into pung-updateChoosePasswordTemplate

4eb1f1a

Merge branch 'master' of https://github.com/ParsePlatform/parse-server

1dc64df

flovilmart suggested changes Jul 8, 2017

View reviewed changes

pungme added 2 commits July 9, 2017 11:34

nits

60ede88

more nits

2d878fb

pungme added 4 commits July 13, 2017 08:45

remove inline-comment

e1b2d61

Merge branch 'master' of https://github.com/ParsePlatform/parse-server …

1466567

…into pung-updateChoosePasswordTemplate

just a merge

66fd2f7

Merge branch 'master' into pung-updateChoosePasswordTemplate

27691ef

montymxb mentioned this pull request Sep 10, 2017

Confirm password on Reset Password #1425

Closed

pungme added 2 commits September 20, 2017 10:51

Merge branch 'master' into pung-updateChoosePasswordTemplate

b15f3fd

Merge branch 'master' into pung-updateChoosePasswordTemplate

91f5670

montymxb approved these changes Nov 3, 2017

View reviewed changes

flovilmart approved these changes Nov 3, 2017

View reviewed changes

flovilmart merged commit 842343a into parse-community:master Nov 3, 2017

parseplatformorg mentioned this pull request Mar 24, 2024

refactor: Upgrade mongodb from 5.9.0 to 6.4.0 #9047

Closed

Uh oh!

Add password confirmation to choose_password #3994

Add password confirmation to choose_password #3994

Uh oh!

Conversation

pungme commented Jul 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jul 4, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

flovilmart left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flovilmart commented Jul 8, 2017

Uh oh!

pungme commented Jul 9, 2017

Uh oh!

flovilmart commented Jul 10, 2017

Uh oh!

pungme commented Jul 10, 2017

Uh oh!

pungme commented Jul 13, 2017

Uh oh!

montymxb commented Sep 10, 2017

Uh oh!

pungme commented Sep 10, 2017

Uh oh!

montymxb commented Nov 3, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flovilmart commented Nov 3, 2017

Uh oh!

flovilmart commented Nov 3, 2017

Uh oh!

montymxb commented Nov 3, 2017

Uh oh!

Uh oh!

pungme commented Jul 4, 2017 •

edited

Loading

codecov bot commented Jul 4, 2017 •

edited

Loading

montymxb commented Nov 3, 2017 •

edited

Loading