Add remark feature #63

erv-teo · 2024-03-29T19:38:49Z

Closes #38

codecov · 2024-03-30T12:53:51Z

Codecov Report

Attention: Patch coverage is 89.53488% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 76.88%. Comparing base (011b226) to head (063f62a).
Report is 6 commits behind head on master.

Files	Patch %	Lines
...c/main/java/seedu/address/model/person/Person.java	44.44%	2 Missing and 3 partials ⚠️
...java/seedu/address/logic/commands/EditCommand.java	90.90%	0 Missing and 1 partial ⚠️
.../seedu/address/logic/parser/AddressBookParser.java	0.00%	1 Missing ⚠️
...c/main/java/seedu/address/model/person/Remark.java	91.66%	1 Missing ⚠️
src/main/java/seedu/address/ui/PersonCard.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master      #63      +/-   ##
============================================
+ Coverage     76.18%   76.88%   +0.70%     
- Complexity      474      496      +22     
============================================
  Files            75       78       +3     
  Lines          1503     1579      +76     
  Branches        154      164      +10     
============================================
+ Hits           1145     1214      +69     
- Misses          314      318       +4     
- Partials         44       47       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

iamtr

Thank you for your work Ervin, it definitely did not seem easy.

Just have a few questions that I'd like to clarify before merging:

Is Remark compulsory in your implementation? Because from what I understand, it is optional.
There is a missing RemarkCommandParserTest for your implementation. You can refer to AddCommandParserTest and add some tests for this.

iamtr · 2024-03-31T07:37:40Z

src/main/java/seedu/address/logic/Messages.java

        person.getTags().forEach(builder::append);
+        builder.append("\n Remarks: ")
+                .append(person.getRemark());
        return builder.toString();


Just a nit, but do you wanna do this instead?

public static String format(Person person) { final StringBuilder builder = new StringBuilder(); builder.append(person.getName()) .append("\n IC: ") .append(person.getIc()) .append("\n DOB: ") .append(person.getDob()) .append("\n Ward: ") .append(person.getWard()) .append("\n Admitted: ") .append(person.getAdmissionDate()) .append("\n Remarks: ") .append(person.getRemark()); .append("\n Tags: "); person.getTags().forEach(builder::append); return builder.toString();

iamtr · 2024-03-31T07:47:23Z

src/test/java/seedu/address/logic/commands/RemarkCommandTest.java

+ */
+public class RemarkCommandTest {
+
+    private static final String REMARK_STUB = "Some remark";


I may be wrong, but I feel like this would be more appropriately named SAMPLE_REMARK instead. Stub implies that it might be a module that is being substituted

iamtr · 2024-03-31T07:52:26Z

src/test/java/seedu/address/testutil/TypicalPersons.java

-            .withAdmissionDate("01/03/2024").withWard("A1").build();
+            .withAdmissionDate("01/03/2024").withWard("A1").withRemark("This is a remark.").build();
    public static final Person BENSON = new PersonBuilder().withName("Benson Meier")
            .withTags("Dementia", "FallRisk").withDob("20/02/2001")
-            .withIc("A2345678B").withAdmissionDate("01/02/2024").withWard("B1").build();
+            .withIc("A2345678B").withAdmissionDate("01/02/2024").withWard("B1").withRemark("This is a remark.").build();
    public static final Person CARL = new PersonBuilder().withName("Carl Kurz").withDob("20/02/2002")
-            .withIc("A3456789B").withAdmissionDate("05/03/2024").withWard("C1").build();
+            .withIc("A3456789B").withAdmissionDate("05/03/2024").withWard("C1").withRemark("This is a remark.").build();
    public static final Person DANIEL = new PersonBuilder().withName("Daniel Meier").withTags("HearingImpaired")
            .withDob("20/02/2003").withIc("A5678901B").withAdmissionDate("10/03/2024")
-            .withWard("D1").build();
+            .withWard("D1").withRemark("This is a remark.").build();
    public static final Person ELLE = new PersonBuilder().withName("Elle Meyer").withDob("20/02/2004")
-            .withIc("A4756976B").withAdmissionDate("05/02/2024").withWard("E1").build();
+            .withIc("A4756976B").withAdmissionDate("05/02/2024").withWard("E1").withRemark("This is a remark.").build();
    public static final Person FIONA = new PersonBuilder().withName("Fiona Kunz").withDob("20/02/2006")
-            .withIc("A5739485B").withAdmissionDate("11/03/2024").withWard("F1").build();
+            .withIc("A5739485B").withAdmissionDate("11/03/2024").withWard("F1").withRemark("This is a remark.").build();
    public static final Person GEORGE = new PersonBuilder().withName("George Best").withDob("20/05/2000")
-            .withIc("A0987654B").withAdmissionDate("16/03/2024").withWard("G1").build();
+            .withIc("A0987654B").withAdmissionDate("16/03/2024").withWard("G1").withRemark("This is a remark.").build();


Is a remark compulsory? Is it possible to make it optional, just like tags?

Maybe we can use more relevant remarks for this part, such as "Patient has limited vision capabilities"

iamtr · 2024-03-31T08:04:39Z

src/main/java/seedu/address/logic/commands/RemarkCommand.java

+/**
+ * Changes the remark of an existing person in the address book.
+ */


Does this allow adding of remarks? More detail here would be good

iamtr · 2024-03-31T08:06:21Z

src/main/java/seedu/address/model/person/Person.java

-                  Dob dob, Ic ic, AdmissionDate admissionDate, Ward ward) {
-        requireAllNonNull(name, dob, ic, admissionDate, ward);
+                  Dob dob, Ic ic, AdmissionDate admissionDate, Ward ward, Remark remark) {
+        requireAllNonNull(name, dob, ic, admissionDate, ward, remark, tags);


I believe remarks was optional, unless I am wrong. Should we exclude remarks from requireNonNull?

iamtr · 2024-03-31T08:10:18Z

src/main/java/seedu/address/model/util/SampleDataUtil.java

-                    new Ic("S1234567A"), new AdmissionDate("05/01/2022"), new Ward("WE")),
+                    new Ic("S1234567A"), new AdmissionDate("05/01/2022"), new Ward("WE"), new Remark("")),
            new Person(new Name("Roy Balakrishnan"), getTagSet("Dementia"), new Dob("01/01/1990"),
-                    new Ic("S1234567A"), new AdmissionDate("06/01/2022"), new Ward("WF"))
+                    new Ic("S1234567A"), new AdmissionDate("06/01/2022"), new Ward("WF"), new Remark(""))


One or two of sample person can have a sample remark too, instead of blank

iamtr · 2024-03-31T08:31:14Z

src/main/java/seedu/address/logic/parser/AddCommandParser.java

        AdmissionDate admissionDate =
                ParserUtil.parseAdmissionDate(argMultimap.getValue(PREFIX_ADMISSION_DATE).orElse(null));
        Ward ward = ParserUtil.parseWard(argMultimap.getValue(PREFIX_WARD).orElse(null));
-
-
-        Person person = new Person(name, tagList, dob, ic, admissionDate, ward);
+        Remark remark = new Remark("");
+        Person person = new Person(name, tagList, dob, ic, admissionDate, ward, remark);


Are we having a parseRemark() method for this? Or remarks have to be added seperately.

iamtr

LGTM. You may wanna add RemarkTest.java too, but that might be optional.

@ryanlimdx @ejnan @swtan346 is there any feedback? If not, we'll merge it soon, so we can test for potential bugs.

erv-teo added 9 commits March 27, 2024 19:08

Recognize remark command

cbfa2b9

Remark throws not implemented error

e627363

Add placeholder for remark

e7c5af7

Modify Person to support Remark

a996065

Change remark string in PersonCard

1981250

Finish remark implementation

5c7cf97

Add remark to person in PersonUtil

9d16707

Add dummy contacts

45cfaed

Fix message usage

6b97aef

erv-teo added this to the v1.3 milestone Mar 29, 2024

erv-teo self-assigned this Mar 29, 2024

erv-teo and others added 4 commits March 30, 2024 03:41

Merge branch 'master' into branch-remarks

bd61835

Fix checkstyle errors

a1afb90

Fix checkstyle errors

69e9b55

Fix failing test cases

ff45099

iamtr reviewed Mar 31, 2024

View reviewed changes

erv-teo and others added 8 commits March 31, 2024 17:38

Allow user to add remark when adding new patient

b7e319e

Change sample data

63de80d

Merge branch 'master' into branch-remarks

9ea25ea

Fix result description

8f33c5d

Add test cases RemarkCommandParser

3b9ac9c

Change naming

ca9c55b

Change remarks of TypicalPersons

8a97fe7

Edit AddCommand message usage to include Remark

d47a4b0

iamtr approved these changes Apr 1, 2024

View reviewed changes

erv-teo added 2 commits April 1, 2024 22:15

Add testcases for Remark

ddbf058

Fix duplicate remark prefixes bug and add test case

063f62a

iamtr merged commit bc449a8 into AY2324S2-CS2103T-F10-1:master Apr 2, 2024
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add remark feature #63

Add remark feature #63

erv-teo commented Mar 29, 2024

codecov bot commented Mar 30, 2024 •

edited

Loading

iamtr left a comment

iamtr Mar 31, 2024

iamtr Mar 31, 2024

iamtr Mar 31, 2024

iamtr Mar 31, 2024

iamtr Mar 31, 2024

iamtr Mar 31, 2024

iamtr Mar 31, 2024

iamtr left a comment

Add remark feature #63

Add remark feature #63

Conversation

erv-teo commented Mar 29, 2024

codecov bot commented Mar 30, 2024 • edited Loading

Codecov Report

iamtr left a comment

Choose a reason for hiding this comment

iamtr Mar 31, 2024

Choose a reason for hiding this comment

iamtr Mar 31, 2024

Choose a reason for hiding this comment

iamtr Mar 31, 2024

Choose a reason for hiding this comment

iamtr Mar 31, 2024

Choose a reason for hiding this comment

iamtr Mar 31, 2024

Choose a reason for hiding this comment

iamtr Mar 31, 2024

Choose a reason for hiding this comment

iamtr Mar 31, 2024

Choose a reason for hiding this comment

iamtr left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 30, 2024 •

edited

Loading