test: Add missing test for Parse.User.verifyPassword option ignoreEmailVerification

[dplewis]

Pull Request

• Report security issues confidentially.
• Any contribution is under this license.
• Link this pull request to an issue.

Issue

#2076 introduced a new feature for verifiying user passwords with a ignoreEmailVerification option. A unit test is missing.

Closes: #2101

Approach

Add test

Tasks

• Add tests
• Add changes to documentation (guides, repository pages, code comments)

[parse-github-assistant]

Thanks for opening this pull request!

[dplewis]

@mtrezza Looks like CodeCov is broken

{'detail': ErrorDetail(string='Unable to locate build via Github Actions API. Please upload with the Codecov repository upload token to resolve issue.', code='not_found')}

[mtrezza]

I assumed an integration test would cover the REST controller as well... why do we need to add a test for the REST controller?

[dplewis]

It was missing coverage https://app.codecov.io/gh/parse-community/Parse-SDK-JS/commit/b0adf7e02ab0beea2cd9b759d0f788c69d291491/blob/src/RESTController.js#L254

Also any particular reason why you added this options to the payload?

[mtrezza]

Looks like CodeCov is broken

Fixed in #2105

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (72bc9ac) to head (09f53f0).
Report is 2 commits behind head on alpha.

Additional details and impacted files

@@             Coverage Diff             @@
##            alpha     #2103      +/-   ##
===========================================
+ Coverage   99.98%   100.00%   +0.01%     
===========================================
  Files          61        61              
  Lines        6185      6185              
  Branches     1499      1499              
===========================================
+ Hits         6184      6185       +1     
+ Misses          1         0       -1     

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

[dplewis]

@mtrezza Nice! Back to 100.00% coverage

[mtrezza]

Also any particular reason why you added this options to the payload?

Where do you suggest to put it?

[dplewis]

Just the placement seemed off. It should have been added to the data send which would have automatically added to the payload. In this case the test wouldn't have been needed.

https://github.com/parse-community/Parse-SDK-JS/blob/alpha/src/RESTController.js#L215-L220

[mtrezza]

I believe I've added it to the options arg because the method was already designed like that:

Parse-SDK-JS/src/ParseUser.js

Lines 1263 to 1266 in c591895

	verifyPassword(username: string, password: string, options: RequestOptions) {
	const RESTController = CoreManager.getRESTController();
	return RESTController.request('GET', 'verifyPassword', { username, password }, options);
	},

username and password are data, and ignoreEmailVerification is an option. Hence it looked odd to me to extract it from the option arg and move it to the data arg. I believe that's why I've decided to pass it through as is.

You think it should be changed?

[dplewis]

I think it should change as you are still extracting it to the data payload in the RESTController. Would this work with directAccess on the server as the RESTController would change? Also there isn't any documentation for this field.

[mtrezza]

Alright then I'll change this in another PR and add the docs.