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
Ask before deleting hardware token #1937
Ask before deleting hardware token #1937
Conversation
fixes this issue - if user wants to delete token which is hardwaretype, he will be asked. Closes privacyidea#954
Codecov Report
@@ Coverage Diff @@
## master #1937 +/- ##
=======================================
Coverage 97.13% 97.13%
=======================================
Files 153 153
Lines 18556 18556
=======================================
Hits 18024 18024
Misses 532 532 Continue to review full report at Codecov.
|
final fix Closes privacyidea#954
05ab07d
to
e0b28aa
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your pull request :-) I have made a few comments.
}; | ||
}; | ||
|
||
$scope.deleteToken = function(){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is pointless. $scope.delete()
already does the exact same thing. Please use $scope.delete().
|
||
$scope.deleteToken = function(){ | ||
TokenFactory.delete($scope.tokenSerial, $scope.return_to, | ||
function (data) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Explanation: TokenFactory.delete
only takes two parameters. The third parameter you have added here is ignored completely. This makes your new function identical to $scope.delete()
.
$scope.delete = function () { | ||
TokenFactory.delete($scope.tokenSerial, $scope.return_to); | ||
$scope.deleteTokenAsk = function() { | ||
$('#dialogTokenDelete').modal(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this line needs to be removed. My suspicion is, that is would cause the modal to always show, even if the token is not a hardware token.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i tested it in venv and it does work as intended.
$('#dialogTokenDelete').modal(); | ||
var tokenType = $scope.token.info.tokenkind; | ||
if (tokenType == "hardware"){ | ||
$('#dialogTokenDelete').modal(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, but please fix the indentation. It is really hard to read otherwise. the code should look something like this:
if (condition) {
doSomething();
} else {
doSomethingElse();
}
if (tokenType == "hardware"){ | ||
$('#dialogTokenDelete').modal(); | ||
} else { | ||
TokenFactory.delete($scope.tokenSerial, $scope.return_to, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The third parameter here does not do anything (see explanation below). What are you trying to do here? We already have a function $scope.delete()
that calls TokenFactory.delete()
with the correct parameters. Unless there is a specific reason why you need the extra code, please just call $scope.delete()
.
<button type="button" | ||
class="btn btn-danger" | ||
data-dismiss="modal" | ||
ng-click="deleteToken()"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As stated above, there is not need for a new function deleteToken()
, as that is what delete()
already does. Please refactor this to use delete()
.
deleted the delete token function and rearranged the indentation.
fixes this issue - if user wants to delete token which is hardwaretype, he will be asked.
Closes #954