Skip to content

Commit

Permalink
Merge branch 'master' of https://github.com/AY2324S1-CS2103T-W09-4/tp
Browse files Browse the repository at this point in the history
…into branch-release

* 'master' of https://github.com/AY2324S1-CS2103T-W09-4/tp:
  Enlarge diagrams
  Fix nits
  Fix minor formatting issue
  Remove JsonAdaptedTag
  UG for filter
  DG: update search sequence diagram
  Checkstyle
  Updated UG
  Remove tests
  Bug Fix for assessment tags and UG update
  Bugfix: add and edit using tags that haven't been created
  UG and DG updates
  Update UG
  Update UG and DG
  Bugfix: editing scores
  • Loading branch information
Misra Aditya committed Nov 14, 2023
2 parents f116e9a + 7b1d7a6 commit 1d4e399
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 126 deletions.
18 changes: 12 additions & 6 deletions docs/DeveloperGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@

This is based on the AddressBook-Level3 project created by the [SE-EDU initiative](https://se-education.org).

_{ list here sources of all reused/adapted ideas, code, documentation, and third-party libraries -- include links to the original source as well }_

--------------------------------------------------------------------------------------------------------------------

Expand Down Expand Up @@ -304,7 +303,7 @@ This is done by setting the `isView` property to true in the `CommandResult` obj

### Create feature

### Implementation
#### Implementation

The `create` feature is implemented using the `CreateTagCommand` class. It extends `Command` and overrides the `execute()` method
to create tags of specific categories.
Expand Down Expand Up @@ -346,15 +345,15 @@ The following sequence diagram shows how the search operation works:

**Note:** The lifeline for `FindCommand` and `FindCommandParser` should end at the destroy marker (X) but due to a limitation of PlantUML, the lifeline reaches the end of diagram.

<puml src="diagrams/SearchSequenceDiagram.puml" width="550" />
<puml src="diagrams/SearchSequenceDiagram.puml" alt="SearchSequenceDiagram"/>

Step 3. The user should see the UI below upon entering `search t/intern`.

![Search](images/search-dg.png)

The following activity diagram shows summarizes what happens when a user attempts to execute the `search` command.

<puml src="diagrams/SearchActivityDiagram.puml" width="550" />
<puml src="diagrams/SearchActivityDiagram.puml" />

**Note:** The current implementation of search allows users to search by any of the categories individually or by different combinations of the categories e.g. `search n/alex bernice st/offered t/intern`
It also allows users to specify more than one search parameter for each category e.g. `search n/alex bernice`
Expand Down Expand Up @@ -405,7 +404,7 @@ Step 3. Assuming Bernice is the applicant matching the requirements, the user sh

The following activity diagram shows summarizes what happens when a user attempts to execute the `delete` command.

<puml src="diagrams/DeleteActivityDiagram.puml" width="550" />
<puml src="diagrams/DeleteActivityDiagram.puml" />

**Note:** The current implementation of delete by tags & status allows users to search by any of the categories individually or by different combinations of the categories.
It also allows users to specify more than one delete parameter for each category e.g. `delete t/intern manager`
Expand Down Expand Up @@ -923,7 +922,14 @@ Currently, the UI does not update and will remain unchanged from the previous co

### Improve on `remark` feature
**Improve the remark feature**
Currently, if you use multiple prefix , it is allowed and only the last prefix will be used. We would like to improve on this by only allowing 1 prefix to be used. Thus we verify for duplicate `r/` prefix since it does not make sense to have multiple remarks for the same person.
Currently, if you use multiple prefix , it is allowed and only the last prefix will be used. We would like to improve on this by only allowing 1 prefix to be used. Thus, we verify for duplicate `r/` prefix since it does not make sense to have multiple remarks for the same person.

### Improve on `edit` feature
**Improve on the edit feature**
Currently, for tagging, once you add a tag to a person, it does not update when you add the tag categories to the same tag name using `create`.
This is a feature flaw since a user would expect the tag to be updated and would not need to be re-tag again. We would like to improve on this by updating the tag when the tag category is updated.
That is if you add an extra category to the tag name, the tag will be updated to include the tag category. Thus, a user would not need to re-tag the person again to update the tag category.


## **Appendix: Effort**

Expand Down
60 changes: 37 additions & 23 deletions docs/UserGuide.md

Large diffs are not rendered by default.

27 changes: 27 additions & 0 deletions docs/diagrams/SearchSequenceDiagram.puml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ participant ":AddressBookParser" as AddressBookParser LOGIC_COLOR
participant ":FindCommandParser" as FindCommandParser LOGIC_COLOR
participant "f:FindCommand" as FindCommand LOGIC_COLOR
participant ":CommandResult" as CommandResult LOGIC_COLOR
participant "n:NameContainsKeywordPredicate" as NameContainsKeywordPredicate LOGIC_COLOR
participant "s:StatusContainsKeywordPredicate" as StatusContainsKeywordPredicate LOGIC_COLOR
participant "t:TagContainsKeywordPredicate" as TagContainsKeywordPredicate LOGIC_COLOR

end box

box Model MODEL_COLOR_T1
Expand All @@ -31,10 +35,33 @@ deactivate FindCommandParser
AddressBookParser -> FindCommandParser : parse("search st/offered")
activate FindCommandParser

create NameContainsKeywordPredicate
FindCommandParser -> NameContainsKeywordPredicate
activate NameContainsKeywordPredicate
NameContainsKeywordPredicate --> FindCommandParser : n
deactivate NameContainsKeywordPredicate

create StatusContainsKeywordPredicate
FindCommandParser -> StatusContainsKeywordPredicate
activate StatusContainsKeywordPredicate
StatusContainsKeywordPredicate --> FindCommandParser : s
deactivate StatusContainsKeywordPredicate

create TagContainsKeywordPredicate
FindCommandParser -> TagContainsKeywordPredicate
activate TagContainsKeywordPredicate
TagContainsKeywordPredicate --> FindCommandParser : t
deactivate TagContainsKeywordPredicate

FindCommandParser -> FindCommandParser: getPredicatesList(n, s, t)
activate FindCommandParser
return predicatesList

create FindCommand
FindCommandParser -> FindCommand : findCommand(predicatesList)
activate FindCommand


FindCommand --> FindCommandParser : f
deactivate FindCommand

Expand Down
34 changes: 25 additions & 9 deletions src/main/java/seedu/address/logic/commands/EditCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,9 @@ public CommandResult execute(Model model) throws CommandException {
}

Person personToEdit = lastShownList.get(index.getZeroBased());
Person editedPerson = createEditedPerson(personToEdit, editPersonDescriptor);
Person editedPersonWithoutFilteredScoreList = createEditedPerson(personToEdit, editPersonDescriptor);

containsIllegalTagScore(editedPerson);
Person editedPerson = filterScoreList(editedPersonWithoutFilteredScoreList, personToEdit);

if (!personToEdit.isSamePerson(editedPerson) && model.hasPerson(editedPerson)) {
throw new CommandException(MESSAGE_DUPLICATE_PERSON);
Expand All @@ -110,7 +110,8 @@ public CommandResult execute(Model model) throws CommandException {
}
}
model.updateFilteredEventList(PREDICATE_SHOW_ALL_EVENTS);
return new CommandResult(String.format(MESSAGE_EDIT_PERSON_SUCCESS, Messages.format(editedPerson)), true);
return new CommandResult(
String.format(MESSAGE_EDIT_PERSON_SUCCESS, Messages.format(editedPerson)), true);
}

private void updateScoreList(Person personToEdit, Person editedPerson) {
Expand All @@ -131,22 +132,37 @@ private void updateScoreList(Person personToEdit, Person editedPerson) {
if (!newTags.contains(tag)) {
newScoreList.removeScore(tag);
}

}
editedPerson.setScoreList(newScoreList);
}

private boolean containsIllegalTagScore(Person person) throws CommandException {
Set<Tag> currentTags = person.getTags();
/**
* Filters the score list of the person to edit, such that only the tags that are in the person's tag list
* @param editedPerson
* @param originalPerson
* @return
*/
private Person filterScoreList(Person editedPerson, Person originalPerson) throws CommandException {
Set<Tag> currentTags = editedPerson.getTags();
Set<Tag> previousTags = originalPerson.getTags();
if (currentTags.isEmpty()) {
return false;
return editedPerson;
}
List<Tag> tagsWithScore = person.getScoreList().getTagsWithScore();

Set<Tag> difference = new HashSet<>(currentTags);
difference.removeAll(previousTags);

List<Tag> tagsWithScore = editedPerson.getScoreList().getTagsWithScore();
for (Tag tag : tagsWithScore) {
if (!currentTags.contains(tag)) {
if (!currentTags.contains(tag) && !previousTags.contains(tag)) {
throw new CommandException(Messages.MESSAGE_ILLEGAL_TAG_SCORE);
}
if (!currentTags.contains(tag) && previousTags.contains(tag)) {
editedPerson.getScoreList().removeScore(tag);
}
}
return false;
return editedPerson;
}

/**
Expand Down
7 changes: 5 additions & 2 deletions src/main/java/seedu/address/model/tag/UniqueTagList.java
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,6 @@ public void add(Tag toAdd) {
* @throws ParseException If the tag is not found in the list.
*/
public Tag getTag(String tagName, String tagCategory) throws ParseException {

Optional<Tag> foundTag = internalList.stream()
.filter(tag -> tag.tagName.equals(tagName) && tag.tagCategory.contains(tagCategory))
.findFirst();
Expand All @@ -99,12 +98,16 @@ public Tag getTag(String tagName, String tagCategory) throws ParseException {
return tag;
}
}
throw new ParseException("Tag category does not exist!");
Tag tag = new Tag(tagName, tagCategory);
add(tag);
return tag;
} else if (foundTag.isPresent()) {
// tag category not specified
long occurrence = internalList.stream()
.filter(tag -> tag.tagName.equals(tagName) && tag.tagCategory.contains(tagCategory))
.count();


// if tag occurs more than once in tag list
if (occurrence > 1) {
throw new ParseException("Multiple tags exists with the same name! "
Expand Down
60 changes: 0 additions & 60 deletions src/main/java/seedu/address/storage/JsonAdaptedTag.java

This file was deleted.

6 changes: 0 additions & 6 deletions src/test/java/seedu/address/logic/parser/ParserUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -262,12 +262,6 @@ public void parseSinglePrefixTags_collectionWithNonExistingTags_throwsParseExcep
+ VALID_TAG_2)));
}

@Test
public void parseTags_collectionWIthInvalidTagsCategorySpecified_throwsParseException() {
assertThrows(ParseException.class, () -> ParserUtil.parseTags(Arrays.asList("employment " + VALID_TAG_1, "dept "
+ VALID_TAG_2)));
}

@Test
public void parseTagCategories_null_throwsNullPointerException() {
assertThrows(NullPointerException.class, () -> ParserUtil.parseTagCategories(null));
Expand Down
20 changes: 0 additions & 20 deletions src/test/java/seedu/address/model/tag/UniqueTagListTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
import static org.junit.jupiter.api.Assertions.assertTrue;
import static seedu.address.testutil.Assert.assertThrows;
import static seedu.address.testutil.TypicalTags.TEST_TAG;
import static seedu.address.testutil.TypicalTags.TEST_TAG_CATEGORY_STRING;
import static seedu.address.testutil.TypicalTags.TEST_TAG_NAME_STRING;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -56,24 +54,6 @@ public void getTag_nullTag_throwsNullPointerException() {
assertThrows(NullPointerException.class, () -> uniqueTagList.getTag(null, null));
}

@Test
public void getTag_tagCategoryDoesNotExist_throwsParseException() {
uniqueTagList.add(TEST_TAG);
assertThrows(ParseException.class, () -> uniqueTagList.getTag(TEST_TAG_NAME_STRING, "categoryNotInList"));
}

@Test
public void getTag_tagNameDoesNotExist_throwsParseException() {
uniqueTagList.add(TEST_TAG);
assertThrows(ParseException.class, () -> uniqueTagList.getTag("nameNotInList", TEST_TAG_CATEGORY_STRING));
}

@Test
public void getTag_tagDoesNotExist_throwsParseException() {
System.out.println(uniqueTagList.contains(TEST_TAG));
assertThrows(ParseException.class, () -> uniqueTagList.getTag(TEST_TAG_NAME_STRING, TEST_TAG_CATEGORY_STRING));
}

@Test
public void getTag_multipleTagsAcrossDifferentCategoriesAndCategoryNotSpecified_throwsParseException() {
uniqueTagList.add(new Tag("test", "category1"));
Expand Down

0 comments on commit 1d4e399

Please sign in to comment.