Skip to content
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

exportcert command: Export only if total hours contributed is positive and non-zero #235

Merged
merged 4 commits into from
Nov 7, 2018

Conversation

sharan8
Copy link

@sharan8 sharan8 commented Nov 6, 2018

Previously,

  • Records with 0 hour values would still be included in the exported certificate.
  • Volunteers with all records having 0 hour values (thus no hours contributed at all) would still have their certificates exported.

This is meaningless and doesn't achieve the intended purpose of the certificate (verification and formal record provision).

Thus, the following change was made:

  • ExportCertCommand now goes through an additional layer of filtering through records to surface only the ones with a non-zero, positive hour value. Thus the above two instances are accordingly managed.

RecordContainsNonZeroHourPredicate has been created for this additional layer of filtering.

@sharan8 sharan8 added this to the v1.4 milestone Nov 6, 2018
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 65.912% when pulling 695efa7 on sharan8:exportcert-command into 36515dc on CS2103-AY1819S1-W16-2:master.

@sharan8 sharan8 requested a review from iMarbles November 6, 2018 16:36
@sharan8 sharan8 added the status.Ready The PR is ready to be reviewed. note: remove this label before merging a PR. label Nov 7, 2018
@iMarbles iMarbles merged commit c8f9c6a into CS2103-AY1819S1-W16-2:master Nov 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status.Ready The PR is ready to be reviewed. note: remove this label before merging a PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants