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

Add functionality to whitelist and blacklist commands #156

Merged
merged 12 commits into from
Nov 4, 2024
Merged
34 changes: 28 additions & 6 deletions docs/UserGuide.md
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ Examples:

Allows updating of various statuses of an existing client.

Format: `edit INDEX [n/NAME] [p/PHONE] [e/EMAIL] [a/ADDRESS] [t/TAG]…​
[ps/PROJECT_STATUS] [py/PAYMENT_STATUS] [cs/CLIENT_STATUS] [d/DEADLINE]`
Format: `edit INDEX [n/NAME] [p/PHONE] [e/EMAIL] [a/ADDRESS] [d/DEADLINE]
[t/TAG]…​ [ps/PROJECT_STATUS] [py/PAYMENT_STATUS] [cs/CLIENT_STATUS]`

* `NAME` Acceptable values are same as in add command
* `PHONE_NUMBER` Acceptable values are same as in add command
Expand Down Expand Up @@ -160,7 +160,7 @@ The deadline field will show an `OVERDUE` label if the deadline has passed and t

Finds persons in client list who match parameters specified.

Format: `find [n/NAME] [p/PHONE_NUMBER] [e/EMAIL] [a/ADDRESS] [t/TAG]… [ps/PROJECT_STATUS] [py/PAYMENT_STATUS] [cs/CLIENT_STATUS] [d/DEADLINE]`
Format: `find [n/NAME] [p/PHONE_NUMBER] [e/EMAIL] [a/ADDRESS] [d/DEADLINE] [t/TAG]… [ps/PROJECT_STATUS] [py/PAYMENT_STATUS] [cs/CLIENT_STATUS]`

* All values matched for any parameter are **case-insensitive**.
* The order of the keywords does not matter. e.g. `Hans Bo` will match `Bo Hans`.
Expand All @@ -178,7 +178,18 @@ Examples:

![result for 'find alex david'](images/findAlexDavidResult.png)

### Delete Client Details: `delete`

<div markdown="span" class="alert alert-primary">:bulb: **Tip:**
#### Shortcuts: Finding blacklisted/whitelisted clients

To find all clients that are blacklisted (with no other parameters), the command `blacklist` can be entered.

Similarly, the `whitelist` command can be entered to find all clients who are whitelisted.

_Note: both of these commands need to be entered without any parameters otherwise the app responds with an error message._
</div>

### Delete Client Details : `delete`

Deletes the specified person from Clientele+.

Expand Down Expand Up @@ -212,10 +223,16 @@ Format: `blacklist INDEX`
Examples:
* `blacklist 2` marks the second person in the list as blacklisted

### Whitelist a Client: `whitelist`
<div markdown="span" class="alert alert-primary">:bulb: **Tip:**
Entering `blacklist` on its own without any other parameters will filter and display all clients who have been blacklisted.
</div>

### Whitelist a Client : `whitelist`

Whitelists a previously-blacklisted client.

_Note: a client is considered to be on the whitelist if their client status is **not** "blacklisted"_

Format: `whitelist INDEX cs/NEW_CLIENT_STATUS`

* Changes the status of the client specified by the index `INDEX` from "blacklisted" to `NEW_CLIENT_STATUS`.
Expand All @@ -227,7 +244,11 @@ Examples:
* `whitelist 2 cs/active` whitelists the second person in the list and marks them as an `active` client.
* `whitelist 1 cs/old` whitelists the first person in the list and marks them as an `old` client.

### Sort Client list: `sort`
<div markdown="span" class="alert alert-primary">:bulb: **Tip:**
Entering `whitelist` on its own without any other parameters will filter and display all clients who have been whitelisted.
</div>

### Sort Client list : `sort`

Sorts the client list in ascending order by the specified field.

Expand Down Expand Up @@ -320,6 +341,7 @@ _Details coming soon ..._

1. **When using multiple screens**, if you move the application to a secondary screen, and later switch to using only the primary screen, the GUI will open off-screen. The remedy is to delete the `preferences.json` file created by the application before running the application again.
2. **If you minimize the Help Window** and then run the `help` command (or use the `Help` menu, or the keyboard shortcut `F1`) again, the original Help Window will remain minimized, and no new Help Window will appear. The remedy is to manually restore the minimized Help Window.
3. **When adding/editing a client after filtering**, the application resets to showing _all_ clients, and the filter may need to be applied again. This is to work around the issue that rises when the client that is added or edited may not match the filter and would not be shown to you which makes it confusing to understand if the command has been executed successfully.

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ public class BlacklistCommand extends Command {

public static final String COMMAND_WORD = "blacklist";

public static final String MESSAGE_USAGE = COMMAND_WORD + " INDEX : adds a person to the blacklist."
+ "\nExample: '" + COMMAND_WORD + " 2' blacklists the 2nd client in the list.";
public static final String MESSAGE_USAGE = COMMAND_WORD + " INDEX : adds a client to the blacklist."
+ "\nExample: '" + COMMAND_WORD + " 2' blacklists the 2nd client in the list."
+ "\nAlternate use: entering '" + COMMAND_WORD + "' shows all blacklisted clients.";

public static final String MESSAGE_BLACKLIST_PERSON_SUCCESS = "Blacklisted Person: %1$s";

Expand Down Expand Up @@ -67,7 +68,7 @@ public CommandResult execute(Model model) throws CommandException {

model.setPerson(personToBlacklist, blacklistedPerson);
model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);
return new CommandResult(String.format(MESSAGE_BLACKLIST_PERSON_SUCCESS, Messages.format(personToBlacklist)));
return new CommandResult(String.format(MESSAGE_BLACKLIST_PERSON_SUCCESS, Messages.format(blacklistedPerson)));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package seedu.address.logic.commands;

import static java.util.Objects.requireNonNull;

import seedu.address.commons.core.index.Index;
import seedu.address.logic.parser.FindCommandParser;
import seedu.address.logic.parser.exceptions.ParseException;
import seedu.address.model.Model;

/**
* Lists out all blacklisted clients
*/
public class BlacklistListCommand extends BlacklistCommand {

/**
* Instantiates a {@code BlacklistListCommand} object.
*/
public BlacklistListCommand() {
super(Index.fromZeroBased(0)); // this is irrelevant but necessary
}

@Override
public CommandResult execute(Model model) {
requireNonNull(model);

try {
// make the find command do the hard work
return (new FindCommandParser().parse("find cs/blacklisted")).execute(model);
} catch (ParseException pe) {
return null; // this will never happen

Check warning on line 30 in src/main/java/seedu/address/logic/commands/BlacklistListCommand.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/seedu/address/logic/commands/BlacklistListCommand.java#L29-L30

Added lines #L29 - L30 were not covered by tests

Choose a reason for hiding this comment

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

By any chance would it be good to have a good error message/log message anyways? In case find command changes or other code is modified? But for now, I agree, it won't happen.

Copy link
Author

Choose a reason for hiding this comment

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

This portion of the find command is reliant on the ArgumentPredicate being changed, which is highly unlikely. But in the future I will be decoupling the blacklist command from the find command entirely; this was just a quick addition that I did not want to initially spend too much time on, so I could focus on the test cases and the whitelist command.

}
}
}
23 changes: 15 additions & 8 deletions src/main/java/seedu/address/logic/commands/WhitelistCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ public class WhitelistCommand extends Command {
+ ": removes person from the blacklist and sets their client status to NEW_CLIENT_STATUS."
+ " NEW_CLIENT_STATUS must be 'old', 'active', 'potential' or 'unresponsive'."
+ "\nExample: '" + COMMAND_WORD + " 2 cs/old' sets the client status of the 2nd "
+ "client in the list as 'old' after removing them from the blacklist.";
+ "client in the list as 'old' after removing them from the blacklist."
+ "\nAlternate use: entering '" + COMMAND_WORD + "' will list out all clients NOT on the blacklist";

public static final String MESSAGE_WHITELIST_PERSON_SUCCESS = "Whitelisted Person: %1$s";
public static final String MESSAGE_CANNOT_WHITELIST = "This person is not on the blacklist: %1$s";
public static final String MESSAGE_CANNOT_BLACKLIST = "Cannot blacklist a person to the whitelist.";

private final Index index;
private final ClientStatus clientStatus;
Expand Down Expand Up @@ -71,13 +73,18 @@ public CommandResult execute(Model model) throws CommandException {

Person personToWhitelist = lastShownList.get(index.getZeroBased());
if (personToWhitelist.isBlacklisted()) {
Person whitelistedPerson = createWhitelistedPerson(personToWhitelist);

model.setPerson(personToWhitelist, whitelistedPerson);
model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);

return new CommandResult(String.format(MESSAGE_WHITELIST_PERSON_SUCCESS,
Messages.format(personToWhitelist)));
if (clientStatus.isBlacklisted()) {
// defensive check; this should ideally be caught by the parser itself
throw new CommandException(MESSAGE_CANNOT_BLACKLIST);
} else {
Person whitelistedPerson = createWhitelistedPerson(personToWhitelist);

model.setPerson(personToWhitelist, whitelistedPerson);
model.updateFilteredPersonList(PREDICATE_SHOW_ALL_PERSONS);

return new CommandResult(String.format(MESSAGE_WHITELIST_PERSON_SUCCESS,
Messages.format(whitelistedPerson)));
}
} else {
throw new CommandException(String.format(MESSAGE_CANNOT_WHITELIST,
Messages.format(personToWhitelist)));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package seedu.address.logic.commands;

import static java.util.Objects.requireNonNull;

import seedu.address.commons.core.index.Index;
import seedu.address.logic.Messages;
import seedu.address.model.Model;
import seedu.address.model.person.ArgumentPredicateToFail;
import seedu.address.model.person.ClientStatus;

/**
* Lists out all whitelisted clients
*/
public class WhitelistListCommand extends WhitelistCommand {

/**
* Instantiates a {@code WhitelistListCommand} object.
*
* @param argPredToFail an argument predicate that should fail
*/
public WhitelistListCommand() {
// this is irrelevant but necessary
super(Index.fromZeroBased(0), new ClientStatus("old"));
}

@Override
public CommandResult execute(Model model) {
requireNonNull(model);

// here, the predicate dictates that the client status should be blacklisted;
// hence if the predicate fails (ensured by the ArgumentPredicateToFail object),
// the client is not blacklisted => they are whitelisted
model.updateFilteredPersonList(ArgumentPredicateToFail.getNotBlacklistedPredicate());

Choose a reason for hiding this comment

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

Just for my understanding, how come there is a different execution for whitelistcommand and blacklistcommand? Couldn't it be easier if there was a predicate 'getBlacklistedPredicate' that kept things consistent and reduces Blacklist's dependence on other code? That being said, I think this still works 👍

Copy link
Author

Choose a reason for hiding this comment

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

Yep, I will be decoupling the blacklist command to make it work more like the whitelist command later on, that was just an easy temporary fix.


return new CommandResult(
String.format(Messages.MESSAGE_PERSONS_LISTED_OVERVIEW, model.getFilteredPersonList().size()));
}
}
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
package seedu.address.logic.parser;

import static java.util.Objects.requireNonNull;
// import static java.util.Objects.requireNonNull;
import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;

import seedu.address.commons.core.index.Index;
import seedu.address.logic.commands.BlacklistCommand;
import seedu.address.logic.commands.BlacklistListCommand;
import seedu.address.logic.parser.exceptions.ParseException;

/**
Expand All @@ -13,20 +14,28 @@
public class BlacklistCommandParser implements Parser<BlacklistCommand> {

/**
* Parses the given {@code String} of arguments in the context of the EditCommand
* and returns an EditCommand object for execution.
* Parses the given {@code String} of arguments in the context of the BlacklistCommand
* and returns a BlacklistCommand object for execution.
* @throws ParseException if the user input does not conform the expected format
*/
public BlacklistCommand parse(String args) throws ParseException {
requireNonNull(args);
// requireNonNull(args);
ArgumentMultimap argMultimap = ArgumentTokenizer.tokenize(args);

Index index;
String preamble = argMultimap.getPreamble();

try {
index = ParserUtil.parseIndex(argMultimap.getPreamble());
index = ParserUtil.parseIndex(preamble);
} catch (ParseException pe) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, BlacklistCommand.MESSAGE_USAGE), pe);
if (preamble.equals("")) {
// only allow "blacklist" with no params as the valid command
return new BlacklistListCommand();
} else {
// any non-empty non-index params throws an error
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT,
BlacklistCommand.MESSAGE_USAGE), pe);
}
}

return new BlacklistCommand(index);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import seedu.address.commons.core.index.Index;
import seedu.address.logic.commands.WhitelistCommand;
import seedu.address.logic.commands.WhitelistListCommand;
import seedu.address.logic.parser.exceptions.ParseException;
import seedu.address.model.person.ClientStatus;

Expand All @@ -17,15 +18,16 @@ public class WhitelistCommandParser implements Parser<WhitelistCommand> {
/**
* Checks if the new client status is valid.
* The new client status must be 'old', 'active' or 'potential',
* it cannot be 'blaclisted' (despite that being a valid client status).
* it cannot be 'blacklisted' (despite it being a valid client status).
* {@code clientStatus} will already be validated.
*/
public boolean isValidNewClientStatus(ClientStatus clientStatus) {
return !clientStatus.isBlacklisted();
}

/**
* Parses the given {@code String} of arguments in the context of the EditCommand
* and returns an EditCommand object for execution.
* Parses the given {@code String} of arguments in the context of the WhitelistCommand
* and returns a WhitelistCommand object for execution.
* @throws ParseException if the user input does not conform the expected format
*/
public WhitelistCommand parse(String args) throws ParseException {
Expand All @@ -35,21 +37,27 @@ public WhitelistCommand parse(String args) throws ParseException {

Index index;
ClientStatus newClientStatus;
String preamble = argMultimap.getPreamble();

try {
index = ParserUtil.parseIndex(argMultimap.getPreamble());
index = ParserUtil.parseIndex(preamble);
} catch (ParseException pe) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, WhitelistCommand.MESSAGE_USAGE), pe);
if (preamble.equals("") && !argMultimap.getValue(PREFIX_CLIENT_STATUS).isPresent()) {
return new WhitelistListCommand();
} else {
throw new ParseException(
String.format(MESSAGE_INVALID_COMMAND_FORMAT, WhitelistCommand.MESSAGE_USAGE), pe);
}
}

if (argMultimap.getValue(PREFIX_CLIENT_STATUS).isPresent()) {
newClientStatus = ParserUtil.parseClientStatus(argMultimap.getValue(PREFIX_CLIENT_STATUS).get());
} else {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, WhitelistCommand.MESSAGE_USAGE));
}

if (isValidNewClientStatus(newClientStatus)) {
return new WhitelistCommand(index, newClientStatus);
if (isValidNewClientStatus(newClientStatus)) {
return new WhitelistCommand(index, newClientStatus);
} else {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, WhitelistCommand.MESSAGE_USAGE));
}
} else {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, WhitelistCommand.MESSAGE_USAGE));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ public ArgumentPredicate(Map<String, Object> parameterMap) {
inputParameters = parameterMap;
}

public Map<String, Object> getInputParameters() {
return this.inputParameters;
}

private void addIfPresent(String key, Predicate<Person> predicate,
Person person, List<Boolean> validParameters) {
assert key != null : "Key should not be null";
Expand Down
Loading