Sciper checking

[mamine2207]

No description provided.

[coveralls]

Pull Request Test Coverage Report for Build 3452171632

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 3 unchanged lines in 1 file lost coverage.
• Overall coverage decreased (-0.05%) to 56.825%

---

Files with Coverage Reduction	New Missed Lines	%
services/dkg/pedersen/handler.go	3	87.23%

Totals
Change from base Build 3412119124:	-0.05%
Covered Lines:	3272
Relevant Lines:	5758

---

💛 - Coveralls

[pierluca]

Nice little PR. I've added a few suggestions for improvement.

[pierluca]

In general, you can reduce indentation by using guard clauses and making the code more readable.
Just a suggestion:

Suggested change

	// The sciper has to contain 6 numbers
	if (sciper < 999999 && sciper > 100000) {
	// call https://search-api.epfl.ch/api/ldap?q=228271 if the answer
	// empty then sciper invalid
	axios.get(`https://search-api.epfl.ch/api/ldap?q=${sciper}`).then((response) => {
	if (response.data.length === 0) {
	res.status(400).send('Unknown sciper');
	} else {
	usersDB.put(sciper, role).catch((error) => {
	res.status(500).send('Failed to add role');
	console.log(error);
	});
	}
	});
	} else {
	res.status(400).send('sciper length incorrect');
	}
	// The sciper has to contain 6 numbers
	if (sciper > 999999 || sciper < 100000) {
	res.status(400).send('sciper length incorrect');
	return;
	}
	// call https://search-api.epfl.ch/api/ldap?q=228271
	// if the answer is empty then the sciper is invalid
	axios.get(`https://search-api.epfl.ch/api/ldap?q=${sciper}`).then((response) => {
	if (response.data.length === 0) {
	res.status(400).send('Unknown sciper');
	} else {
	usersDB.put(sciper, role).catch((error) => {
	res.status(500).send('Failed to add role');
	console.log(error);
	});
	}
	});

[pierluca]

what if the get fails ? it seems you don't catch that case.

[mamine2207]

Yes I missed this one, I just added a catch in a new commit

[pierluca]

For you information, you can also concatenate promises, by returning a promise from within the body of a then.

This is sometimes more readable, and you can group all the errors in one catch.

Suggested change

	axios.get(`https://search-api.epfl.ch/api/ldap?q=${sciper}`).then((response) => {
	if (response.data.length === 0) {
	res.status(400).send('Unknown sciper');
	} else {
	usersDB.put(sciper, role).catch((error) => {
	res.status(500).send('Failed to add role');
	console.log(error);
	});
	}
	});
	axios.get(`https://search-api.epfl.ch/api/ldap?q=${sciper}`)
	.then((response) => {
	if (response.data.length === 0) {
	res.status(400).send('Unknown sciper');
	return;
	}
	return usersDB.put(sciper, role);
	})
	.catch((error) => {
	res.status(500).send('Failed to add role');
	console.log(error);
	});;

[pierluca]

this will also execute in the case of the catch above, don't you want to put in a then (before the catch) ?

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication