-
Notifications
You must be signed in to change notification settings - Fork 6
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 processing of multiple groups #144
Conversation
@@ -170,7 +170,6 @@ protected function execute(InputInterface $input, OutputInterface $output) { | |||
} | |||
} else { | |||
$output->writeln("Ignoring missing group $group"); | |||
return 1; |
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.
@PVince81 what should happen here?
Since we may have already processed some groups, I think we should continue on and process any other groups.
But then after looping through all the groups, should we remember that there was a problem with some group, and exit with non-zero status?
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.
yeah, sounds good. continue and report at the end
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.
fix now or create ticket for later for the additional reporting ?
@PVince81 please review. This makes it try to process any mix of groups and UIDs given on the command line. |
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.
Makes sense. Next up would be updating the unit tests
Is there anyone who can do that quickly? I am not an expert in that area. And I should finish a couple of other password policy manual tests. |
@sharidas or whoever does unit tests, it will need to test some of:
|
Unit test update to fix processing of multiple groups Signed-off-by: Sujith H <sharidasan@owncloud.com>
Codecov Report
@@ Coverage Diff @@
## master #144 +/- ##
============================================
+ Coverage 72.26% 72.83% +0.56%
- Complexity 217 225 +8
============================================
Files 25 25
Lines 952 968 +16
============================================
+ Hits 688 705 +17
+ Misses 264 263 -1
Continue to review full report at Codecov.
|
Fixes scenarios of issue #143
The erroneous
Invalid argument given.
message is no longer shown when expiring passwords for a group.When one of the group names is not a valid group, the command continues processing the remaining groups.
When one of the group names is not a valid group, the command continues processing the remaining groups.
At the end of processing, the totals of any missing groups or users are reported, and the command returns with non-zero status.