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

Batch student status update #62

Conversation

joshuayapwj98
Copy link

No description provided.

joshuayapwj98 and others added 7 commits March 8, 2022 16:06
files edited:
- EditCommand.java: added status and classCode in isAnyFieldEdited() method to cater for the 2 fields.
Edited files:
1. EditCommand.java: Added comments to document approach to the feature.
and findclasscode
- Update own PPP to record new changes
…t-command

Add checks for Status and ClassCode
@lzf834 lzf834 added priority.Low Low Priority Tasks type.AdvancedFeatures Advanced Features that we plan to implement in the upcoming versions type.Epic A big feature which can be broken down into smaller stories e.g. search labels Mar 8, 2022
@lzf834
Copy link

lzf834 commented Mar 8, 2022

Checkstyle:

  • Minor changes for the new file created, need new line at EOF in update_student_in_batches.

@Fenway17
Copy link

Fenway17 commented Mar 8, 2022

Minor checkstyle errors detected by Java CI.

@whoisjunhong
Copy link

whoisjunhong commented Mar 12, 2022

Closed since changes have been made and updated in #70 and 71

@joshuayapwj98 joshuayapwj98 reopened this Mar 12, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #62 (034b174) into master (1e0d165) will decrease coverage by 0.96%.
The diff coverage is 44.92%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master      #62      +/-   ##
============================================
- Coverage     72.22%   71.25%   -0.97%     
- Complexity      419      439      +20     
============================================
  Files            72       78       +6     
  Lines          1314     1381      +67     
  Branches        141      152      +11     
============================================
+ Hits            949      984      +35     
- Misses          320      353      +33     
+ Partials         45       44       -1     
Impacted Files Coverage Δ
...u/address/logic/commands/FindClassCodeCommand.java 0.00% <0.00%> (ø)
.../seedu/address/logic/parser/AddressBookParser.java 84.21% <0.00%> (-4.03%) ⬇️
...dress/logic/parser/FindClassCodeCommandParser.java 0.00% <0.00%> (ø)
...del/person/ClassCodeContainsKeywordsPredicate.java 0.00% <0.00%> (ø)
...java/seedu/address/logic/commands/EditCommand.java 82.60% <13.33%> (-12.33%) ⬇️
...eedu/address/logic/commands/FindStatusCommand.java 100.00% <100.00%> (ø)
.../address/logic/parser/FindStatusCommandParser.java 100.00% <100.00%> (ø)
...c/main/java/seedu/address/model/person/Status.java 83.33% <100.00%> (+3.33%) ⬆️
.../model/person/StatusContainsKeywordsPredicate.java 100.00% <100.00%> (ø)
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1e0d165...034b174. Read the comment docs.

Comment on lines +95 to +110
if (personToEdit.getStatus().toString().equals(Status.NEGATIVE)
&& editedPerson.getStatus().toString().equals(Status.POSITIVE)) {

List<Person> filteredByClassCodeList = studentList.stream()
.filter(student -> student.getClassCode().toString().equals(editedPerson.getClassCode().toString())
&& !student.isSamePerson(editedPerson))
.collect(Collectors.toList());

for (int i = 0; i < filteredByClassCodeList.size(); i++) {
Person currentPerson = filteredByClassCodeList.get(i);
EditPersonDescriptor tempDescriptor = new EditPersonDescriptor();
tempDescriptor.setStatus(new Status(Status.CLOSE_CONTACT));
Person editedPersonStatus = createEditedPerson(currentPerson, tempDescriptor);
model.setPerson(currentPerson, editedPersonStatus);
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we also handle the case where a student's status has changed from positive to negative? When a student has recovered, their classmates' status should be negative instead of close contact if there is no more positive student in the class.

@louisdavinlie
Copy link

Looks good!

@louisdavinlie louisdavinlie merged commit bc0d122 into AY2122S2-CS2103T-T12-1:master Mar 12, 2022
@lzf834
Copy link

lzf834 commented Mar 14, 2022

Might need to consider an edge case where already Positive student should not be changed to close-contact upon update of a different student

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority.Low Low Priority Tasks type.AdvancedFeatures Advanced Features that we plan to implement in the upcoming versions type.Epic A big feature which can be broken down into smaller stories e.g. search
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants