Skip to content

Commit

Permalink
Remove organization RID parsing and JSON file storage
Browse files Browse the repository at this point in the history
These have design flaws that will lead to more user confusion and
possible bugs than it's worth. They are removed from here, and
remaining getters should be modified to be automatically determined
in future updates instead.
  • Loading branch information
wxwern committed Oct 19, 2023
1 parent a2f8a4b commit a1b1d8f
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import static seedu.address.logic.parser.CliSyntax.FLAG_PHONE;
import static seedu.address.logic.parser.CliSyntax.FLAG_POSITION;
import static seedu.address.logic.parser.CliSyntax.FLAG_RECRUITER;
import static seedu.address.logic.parser.CliSyntax.FLAG_RECRUITER_ID;
import static seedu.address.logic.parser.CliSyntax.FLAG_STATUS;
import static seedu.address.logic.parser.CliSyntax.FLAG_TAG;
import static seedu.address.logic.parser.CliSyntax.FLAG_URL;
Expand Down Expand Up @@ -51,7 +50,7 @@ public AddCommand parse(String args) throws ParseException {
FLAG_ADDRESS, FLAG_TAG, FLAG_URL,
FLAG_ID, FLAG_STATUS, FLAG_POSITION,
FLAG_ORGANIZATION_ID,
FLAG_ORGANIZATION, FLAG_RECRUITER, FLAG_RECRUITER_ID
FLAG_ORGANIZATION, FLAG_RECRUITER
);

if (!argMultimap.hasAllOfFlags(FLAG_NAME)
Expand Down Expand Up @@ -136,7 +135,7 @@ private Organization parseAsOrganization(ArgumentMultimap argMultimap) throws Pa
Status status = ParserUtil.parseOptionally(
argMultimap.getValue(FLAG_STATUS), ParserUtil::parseStatus);
Set<Tag> tagList = ParserUtil.parseTags(argMultimap.getAllValues(FLAG_TAG));
Set<Id> ridList = ParserUtil.parseIds(argMultimap.getAllValues(FLAG_RECRUITER_ID));
Set<Id> ridList = Set.of(); // TODO: This should be dynamically determined from oid in Recruiter.

return new Organization(name, id, phone, email, url, address, tagList, status, position, ridList);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/seedu/address/logic/parser/CliSyntax.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public class CliSyntax {
public static final Flag FLAG_ID = new Flag("id");
public static final Flag FLAG_RECURSIVE = new Flag("recursive");
public static final Flag FLAG_ORGANIZATION_ID = new Flag("oid");
public static final Flag FLAG_RECRUITER_ID = new Flag("PUT-ON-PROBATION");
public static final Flag FLAG_RECRUITER_ID = new Flag("rid");


}
19 changes: 3 additions & 16 deletions src/main/java/seedu/address/storage/JsonAdaptedContact.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ class JsonAdaptedContact {
private final String url;
private String oid;
private final List<JsonAdaptedTag> tags = new ArrayList<>();
private final List<JsonAdaptedId> rids = new ArrayList<>();


/**
Expand All @@ -55,8 +54,7 @@ public JsonAdaptedContact(@JsonProperty("type") String type,
@JsonProperty("phone") String phone, @JsonProperty("email") String email,
@JsonProperty("url") String url, @JsonProperty("address") String address,
@JsonProperty("status") String status, @JsonProperty("position") String position,
@JsonProperty("oid") String oid, @JsonProperty("tags") List<JsonAdaptedTag> tags,
@JsonProperty("rid") List<JsonAdaptedId> rids) {
@JsonProperty("oid") String oid, @JsonProperty("tags") List<JsonAdaptedTag> tags) {
this.type = type;
this.name = name;
this.id = id;
Expand All @@ -70,9 +68,6 @@ public JsonAdaptedContact(@JsonProperty("type") String type,
if (tags != null) {
this.tags.addAll(tags);
}
if (rids != null) {
this.rids.addAll(rids);
}
}

/**
Expand All @@ -88,9 +83,6 @@ public JsonAdaptedContact(Contact source) {
.map(position -> position.jobPosition)
.orElse(null);
oid = "";
rids.addAll(((Organization) source).getRecruiterIds().stream()
.map(JsonAdaptedId::new)
.collect(Collectors.toList()));
} else if (source.getType() == Type.RECRUITER) {
Recruiter recruiter = (Recruiter) source;
status = "";
Expand Down Expand Up @@ -124,11 +116,6 @@ public Contact toModelType() throws IllegalValueException {
contactTags.add(tag.toModelType());
}

final List<Id> contactRids = new ArrayList<>();
for (JsonAdaptedId rid : rids) {
contactRids.add(rid.toModelType());
}

// Type#fromString implicitly returns UNKNOWN if type is null. May change if UNKNOWN is removed in the future.
final Type modelType = Type.fromString(type);

Expand Down Expand Up @@ -170,10 +157,10 @@ public Contact toModelType() throws IllegalValueException {

final Set<Tag> modelTags = new HashSet<>(contactTags);

final Set<Id> modelRids = new HashSet<>(contactRids);

switch (modelType) {
case ORGANIZATION: {
final Set<Id> modelRids = new HashSet<>();

if (status != null && !Status.isValidStatus(status)) {
throw new IllegalValueException(Status.MESSAGE_CONSTRAINTS);
}
Expand Down
18 changes: 9 additions & 9 deletions src/test/java/seedu/address/storage/JsonAdaptedContactTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public void toModelType_validContactDetails_returnsContact() throws Exception {
public void toModelType_invalidName_throwsIllegalValueException() {
JsonAdaptedContact person = new JsonAdaptedContact(VALID_TYPE_ORG, INVALID_NAME, VALID_ID,
VALID_PHONE, VALID_EMAIL, VALID_URL, VALID_ADDRESS, VALID_STATUS, VALID_POSITION,
VALID_OID, VALID_TAGS, VALID_RIDS);
VALID_OID, VALID_TAGS);
String expectedMessage = Name.MESSAGE_CONSTRAINTS;
assertThrows(IllegalValueException.class, expectedMessage, person::toModelType);
}
Expand All @@ -70,7 +70,7 @@ public void toModelType_invalidName_throwsIllegalValueException() {
public void toModelType_nullName_throwsIllegalValueException() {
JsonAdaptedContact person = new JsonAdaptedContact(VALID_TYPE_ORG, null, VALID_ID,
VALID_PHONE, VALID_EMAIL, VALID_URL, VALID_ADDRESS, VALID_STATUS, VALID_POSITION,
VALID_OID, VALID_TAGS, VALID_RIDS);
VALID_OID, VALID_TAGS);
String expectedMessage = String.format(MISSING_FIELD_MESSAGE_FORMAT, Name.class.getSimpleName());
assertThrows(IllegalValueException.class, expectedMessage, person::toModelType);
}
Expand All @@ -79,7 +79,7 @@ public void toModelType_nullName_throwsIllegalValueException() {
public void toModelType_invalidPhone_throwsIllegalValueException() {
JsonAdaptedContact person = new JsonAdaptedContact(VALID_TYPE_ORG, VALID_NAME, VALID_ID,
INVALID_PHONE, VALID_EMAIL, VALID_URL, VALID_ADDRESS, VALID_STATUS, VALID_POSITION,
VALID_OID, VALID_TAGS, VALID_RIDS);
VALID_OID, VALID_TAGS);
String expectedMessage = Phone.MESSAGE_CONSTRAINTS;
assertThrows(IllegalValueException.class, expectedMessage, person::toModelType);
}
Expand All @@ -88,15 +88,15 @@ public void toModelType_invalidPhone_throwsIllegalValueException() {
public void toModelType_nullPhone_doesNotThrowException() {
JsonAdaptedContact person = new JsonAdaptedContact(VALID_TYPE_ORG, VALID_NAME, VALID_ID,
null, VALID_EMAIL, VALID_URL, VALID_ADDRESS, VALID_STATUS, VALID_POSITION,
VALID_OID, VALID_TAGS, VALID_RIDS);
VALID_OID, VALID_TAGS);
assertDoesNotThrow(person::toModelType);
}

@Test
public void toModelType_invalidEmail_throwsIllegalValueException() {
JsonAdaptedContact person = new JsonAdaptedContact(VALID_TYPE_ORG, VALID_NAME, VALID_ID,
VALID_PHONE, INVALID_EMAIL, VALID_URL, VALID_ADDRESS, VALID_STATUS, VALID_POSITION,
VALID_OID, VALID_TAGS, VALID_RIDS);
VALID_OID, VALID_TAGS);
String expectedMessage = Email.MESSAGE_CONSTRAINTS;
assertThrows(IllegalValueException.class, expectedMessage, person::toModelType);
}
Expand All @@ -105,15 +105,15 @@ public void toModelType_invalidEmail_throwsIllegalValueException() {
public void toModelType_nullEmail_doesNotThrowException() {
JsonAdaptedContact person = new JsonAdaptedContact(VALID_TYPE_ORG, VALID_NAME, VALID_ID,
VALID_PHONE, null, VALID_URL, VALID_ADDRESS, VALID_STATUS, VALID_POSITION,
VALID_OID, VALID_TAGS, VALID_RIDS);
VALID_OID, VALID_TAGS);
assertDoesNotThrow(person::toModelType);
}

@Test
public void toModelType_invalidAddress_throwsIllegalValueException() {
JsonAdaptedContact person = new JsonAdaptedContact(VALID_TYPE_ORG, VALID_NAME, VALID_ID,
VALID_PHONE, VALID_EMAIL, VALID_URL, INVALID_ADDRESS, VALID_STATUS, VALID_POSITION,
VALID_OID, VALID_TAGS, VALID_RIDS);
VALID_OID, VALID_TAGS);
String expectedMessage = Address.MESSAGE_CONSTRAINTS;
assertThrows(IllegalValueException.class, expectedMessage, person::toModelType);
}
Expand All @@ -122,7 +122,7 @@ public void toModelType_invalidAddress_throwsIllegalValueException() {
public void toModelType_nullAddress_doesNotThrowException() {
JsonAdaptedContact person = new JsonAdaptedContact(VALID_TYPE_ORG, VALID_NAME, VALID_ID,
VALID_PHONE, VALID_EMAIL, VALID_URL, null, VALID_STATUS, VALID_POSITION,
VALID_OID, VALID_TAGS, VALID_RIDS);
VALID_OID, VALID_TAGS);
assertDoesNotThrow(person::toModelType);
}

Expand All @@ -132,7 +132,7 @@ public void toModelType_invalidTags_throwsIllegalValueException() {
invalidTags.add(new JsonAdaptedTag(INVALID_TAG));
JsonAdaptedContact person = new JsonAdaptedContact(VALID_TYPE_ORG, VALID_NAME, VALID_ID,
VALID_PHONE, VALID_EMAIL, VALID_URL, VALID_ADDRESS, VALID_STATUS, VALID_POSITION,
VALID_OID, invalidTags, VALID_RIDS);
VALID_OID, invalidTags);
assertThrows(IllegalValueException.class, person::toModelType);
}

Expand Down

0 comments on commit a1b1d8f

Please sign in to comment.