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

[F11-2] Concierge #68

Open
wants to merge 559 commits into
base: master
Choose a base branch
from

Conversation

adamwth
Copy link

@adamwth adamwth commented Sep 20, 2018

Copy link

@liliwei25 liliwei25 left a comment

Choose a reason for hiding this comment

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

Proceed to work towards v1.1. 👍

@damithc
Copy link

damithc commented Oct 2, 2018

Guys, the current version of your team repo is failing CI checks. Try to fix before v1.1

Copy link

@liliwei25 liliwei25 left a comment

Choose a reason for hiding this comment

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

There seems to some discrepancies in your docs. Please double check.

Great effort! 👍

LICENSE Outdated
@@ -2,11 +2,11 @@ MIT License

Copyright (c) 2016 Software Engineering Education - FOSS Resources

Permission is hereby granted, free of charge, to any person obtaining a copy
Permission is hereby granted, free of charge, to any guest obtaining a copy

Choose a reason for hiding this comment

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

I don't think you're supposed to edit the license

Copy link
Author

Choose a reason for hiding this comment

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

Oops, thanks!

Lists all the available rooms of that type for the next NUM_DAYS days +
Format: `avail r/ROOM TYPE num/NUM_DAYS`

Lists all available rooms from start to end date. Dates have to be given in YYMMDD format

Choose a reason for hiding this comment

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

This is confusing. Is this searched from a start to end date or is this searched for next NUM_DAYS? And where is the YYMMDD date used as input?


Adds a person to the address book +
=== Clearing all entries : `avail`

Choose a reason for hiding this comment

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

is 'avail' used for clearing all entries?


Examples:

* `avail r/Double Room num/5`

Choose a reason for hiding this comment

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

These examples for 'avail' is separated from the 'avail' command description by the 'add' command

* `avail r/Double Room num/5`
* `avail r/Double Room ds/20180910 de/21080912`

=== Listing all guests : `list`

Choose a reason for hiding this comment

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

So are you listing all guests or all room types?

@@ -582,9 +582,9 @@ Do take a look at <<Design-Model>> before attempting to modify the `Model` compo
* Hints
** The link:{repoURL}/src/main/java/seedu/address/model/Model.java[`Model`] and the link:{repoURL}/src/main/java/seedu/address/model/AddressBook.java[`AddressBook`] API need to be updated.
** Think about how you can use SLAP to design the method. Where should we place the main logic of deleting tags?
** Find out which of the existing API methods in link:{repoURL}/src/main/java/seedu/address/model/AddressBook.java[`AddressBook`] and link:{repoURL}/src/main/java/seedu/address/model/person/Person.java[`Person`] classes can be used to implement the tag removal logic. link:{repoURL}/src/main/java/seedu/address/model/AddressBook.java[`AddressBook`] allows you to update a person, and link:{repoURL}/src/main/java/seedu/address/model/person/Person.java[`Person`] allows you to update the tags.
** Find out which of the existing API methods in link:{repoURL}/src/main/java/seedu/address/model/AddressBook.java[`AddressBook`] and link:{repoURL}/src/main/java/seedu/address/model/guest/Person.java[`Person`] classes can be used to implement the tag removal logic. link:{repoURL}/src/main/java/seedu/address/model/AddressBook.java[`AddressBook`] allows you to update a guest, and link:{repoURL}/src/main/java/seedu/address/model/guest/Person.java[`Person`] allows you to update the tags.

Choose a reason for hiding this comment

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

/guest/Person.java or /person/Guest.java?

public static final String MESSAGE_USAGE = COMMAND_WORD
+ ": Assigns a guest to a room.\n"
+ "Parameters: GUEST_NAME ROOM_NUMBER\n"
+ "Example: " + COMMAND_WORD + " Thomas 056";

Choose a reason for hiding this comment

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

This format is different from the one specified in your User docs. Please make it consistent

Guest guestToAssign = lastShownList.get(targetIndex.getZeroBased());
model.commitAddressBook();

return new CommandResult(String.format(MESSAGE_ASSIGN_GUEST_SUCCESS, guestToAssign));

Choose a reason for hiding this comment

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

String accepts two inputs but only one was given

/**
* Returns true if a given int is a valid capacity.
*/
public static boolean isValidCapacity(int test) {

Choose a reason for hiding this comment

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

'test' is not a good variable/parameter name

Copy link

@liliwei25 liliwei25 left a comment

Choose a reason for hiding this comment

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

Good amount of effort! 👍

  • Some classes/methods/variables can be renamed since this is not an addressbook anymore
  • The class diagrams should include the visibility of the attributes
  • Should set up for reposense too

@@ -0,0 +1,42 @@
19/10/2018 06:33:22 a

Choose a reason for hiding this comment

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

Should not add this file to git

. `Room#checkout` will
.. throw `NoBookingException` if the room has no bookings
.. throw `NoActiveOrExpiredBookingException` if the room has no active or expired bookings
.. delete the first booking from the list of bookings

Choose a reason for hiding this comment

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

Does it delete the first booking or booking of room "001"?

string.append(it.next() + newLine);
}
String inputString = string.toString();
System.out.println(inputString);

Choose a reason for hiding this comment

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

extra println statement?

}

@Override
public CommandResult execute(Model model, CommandHistory history) throws CommandException {
requireNonNull(model);

if (model.hasPerson(toAdd)) {
if (model.hasPerson(guestToAdd)) {

Choose a reason for hiding this comment

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

maybe can change method name to "hasGuest" to standardize

throw new CommandException(MESSAGE_DUPLICATE_PERSON);
}
model.addPerson(guestToAdd);

Choose a reason for hiding this comment

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

"addPerson" -> "addGuest"?

@@ -15,7 +21,8 @@
public class AddressBook implements ReadOnlyAddressBook {

Choose a reason for hiding this comment

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

maybe can rename the class as well since this is no longer an addressbook

import seedu.address.model.person.Guest;
import seedu.address.model.person.UniqueGuestList;

/**
* A utility class containing a list of {@code Guest} objects to be used in tests.
*/
public class TypicalPersons {

Choose a reason for hiding this comment

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

Maybe can rename to TypicalGuests?

/**
* A utility class containing a {@code UniqueGuestList} and {@code UniqueRoomList} objects to be used in tests.
*/
public class TypicalAddressBook {

Choose a reason for hiding this comment

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

maybe can rename to something that suits your app?

}

/**
* Initializes the ExpenseBuilder with the data of {@code bookingToCopy}.

Choose a reason for hiding this comment

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

typo

// Identity fields
private final Guest guest;
private final BookingPeriod bookingPeriod;
private final Boolean checkIn;

Choose a reason for hiding this comment

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

check naming convention for booleans

bannified pushed a commit to bannified/addressbook-level4 that referenced this pull request Oct 25, 2018
Bellaaarh pushed a commit to Bellaaarh/addressbook-level4 that referenced this pull request Oct 29, 2018
…shaw_yeong_changes

add AnalyticsCommandTest and update UML Diagrams
teowz46 and others added 30 commits November 12, 2018 10:50
* XmlAdaptedRoom: fix IllegalValueException due to expenses present in a checked-in but expired booking

The current implementation of XmlAdaptedRoom will only allow expenses to be added to rooms that have active and checked-in bookings.

However, this does not reflect the real-life scenario. For example, the user might exit the app on the same day as the last day of a booking with expenses, without checking out the room. The next day, the booking will become expired, and starting the app will thus throw an error. This behaviour should be prevented as it is possible that users may forget to check out after the day.

The section on adding expenses has been adjusted to allow for this. Expenses can now be added as long as there is a booking, and that booking is checked-in.

* SampleDataUtil: add two more expense types into sample

Two expense types were added for the purpose of representing discounts and typo corrections. This is necessary since there is no command for editing or deleting an expense, hence any adjustment will have to be added as an expense with either positive or negative value. These two expense types are thus general expense types for such a purpose.

* UserGuide: update service section with the two adjustment expense types

A description for the usage of the two adjustment expense types has been added.

* UserGuide: fix minor stuff

* DeveloperGuide: add ServiceCommand stuff with a flowchart

Stuff was added. May add more stuff.

* ModelClassDiagram: update with current structure

The ModelClassDiagram illustrated the Address Book structure. The diagram has been updated to reflect our Concierge structure.

* ModelClassDiagram: update the better diagram also

The other diagram was updated with our current terminology. UniqueRoomList and Menu were not included, otherwise it would be too cluttered (and also because I'm lazy).

* DeveloperGuide: add sequence diagram for addExpense

Sequence diagram for the entire process of adding an expense is added. This is to supplement the flowchart with more detail on how an expense is added.

* DeveloperGuide: make addExpense sequence diagram bigger

The words are a bit unreadable.

* DeveloperGuide: add more design considerations

* all: fix checkstyle errors

* docs: add my own PPP

* XmlAdaptedRoomTest: add more tests to increase coverage

Tests targeted at some of the highlighted parts in Coveralls. Not sure why the other highlighted parts were not covered.

* XmlAdaptedRoomTest: fix checkstyle errors

* UserGuide: add minor corrections
* XmlAdaptedRoom: fix IllegalValueException due to expenses present in a checked-in but expired booking

The current implementation of XmlAdaptedRoom will only allow expenses to be added to rooms that have active and checked-in bookings.

However, this does not reflect the real-life scenario. For example, the user might exit the app on the same day as the last day of a booking with expenses, without checking out the room. The next day, the booking will become expired, and starting the app will thus throw an error. This behaviour should be prevented as it is possible that users may forget to check out after the day.

The section on adding expenses has been adjusted to allow for this. Expenses can now be added as long as there is a booking, and that booking is checked-in.

* SampleDataUtil: add two more expense types into sample

Two expense types were added for the purpose of representing discounts and typo corrections. This is necessary since there is no command for editing or deleting an expense, hence any adjustment will have to be added as an expense with either positive or negative value. These two expense types are thus general expense types for such a purpose.

* UserGuide: update service section with the two adjustment expense types

A description for the usage of the two adjustment expense types has been added.

* UserGuide: fix minor stuff

* DeveloperGuide: add ServiceCommand stuff with a flowchart

Stuff was added. May add more stuff.

* ModelClassDiagram: update with current structure

The ModelClassDiagram illustrated the Address Book structure. The diagram has been updated to reflect our Concierge structure.

* ModelClassDiagram: update the better diagram also

The other diagram was updated with our current terminology. UniqueRoomList and Menu were not included, otherwise it would be too cluttered (and also because I'm lazy).

* DeveloperGuide: add sequence diagram for addExpense

Sequence diagram for the entire process of adding an expense is added. This is to supplement the flowchart with more detail on how an expense is added.

* DeveloperGuide: make addExpense sequence diagram bigger

The words are a bit unreadable.

* DeveloperGuide: add more design considerations

* all: fix checkstyle errors

* docs: add my own PPP

* XmlAdaptedRoomTest: add more tests to increase coverage

Tests targeted at some of the highlighted parts in Coveralls. Not sure why the other highlighted parts were not covered.

* XmlAdaptedRoomTest: fix checkstyle errors

* UserGuide: add minor corrections

* UserGuide and DeveloperGuide: minor edits from previous PR

* UserGuide: use important icon

* UserGuide: fix some formatting errors
* XmlAdaptedRoom: fix IllegalValueException due to expenses present in a checked-in but expired booking

The current implementation of XmlAdaptedRoom will only allow expenses to be added to rooms that have active and checked-in bookings.

However, this does not reflect the real-life scenario. For example, the user might exit the app on the same day as the last day of a booking with expenses, without checking out the room. The next day, the booking will become expired, and starting the app will thus throw an error. This behaviour should be prevented as it is possible that users may forget to check out after the day.

The section on adding expenses has been adjusted to allow for this. Expenses can now be added as long as there is a booking, and that booking is checked-in.

* SampleDataUtil: add two more expense types into sample

Two expense types were added for the purpose of representing discounts and typo corrections. This is necessary since there is no command for editing or deleting an expense, hence any adjustment will have to be added as an expense with either positive or negative value. These two expense types are thus general expense types for such a purpose.

* UserGuide: update service section with the two adjustment expense types

A description for the usage of the two adjustment expense types has been added.

* UserGuide: fix minor stuff

* DeveloperGuide: add ServiceCommand stuff with a flowchart

Stuff was added. May add more stuff.

* ModelClassDiagram: update with current structure

The ModelClassDiagram illustrated the Address Book structure. The diagram has been updated to reflect our Concierge structure.

* ModelClassDiagram: update the better diagram also

The other diagram was updated with our current terminology. UniqueRoomList and Menu were not included, otherwise it would be too cluttered (and also because I'm lazy).

* DeveloperGuide: add sequence diagram for addExpense

Sequence diagram for the entire process of adding an expense is added. This is to supplement the flowchart with more detail on how an expense is added.

* DeveloperGuide: make addExpense sequence diagram bigger

The words are a bit unreadable.

* DeveloperGuide: add more design considerations

* all: fix checkstyle errors

* docs: add my own PPP

* XmlAdaptedRoomTest: add more tests to increase coverage

Tests targeted at some of the highlighted parts in Coveralls. Not sure why the other highlighted parts were not covered.

* XmlAdaptedRoomTest: fix checkstyle errors

* UserGuide: add minor corrections

* UserGuide and DeveloperGuide: minor edits from previous PR

* UserGuide: use important icon

* UserGuide: fix some formatting errors

* UserGuide: rearrange stuff in service
* UG: update checkin/out, ContactUs: update email

* PPP: update

* asciidoctor: remove line 381 - fixes html links appearing in pdf

* DG: add update UML diagrams

* fix style

* update name as shown in IVLE

* PPP: add terminology
* Modify find function include both guests and rooms searching

* Change test details, convert prefixes over to new ones.

* Minor format change

* More minor change

* Change for Travis

* Files not in sync, have to commit and check travis again and again

* Update for Travis/Testing

* Replace code to utilize tokenizer

* Replace code to utilize tokenizer

* Made room list appear first, create tests for ListCommand and its parser

* More fixes

* AdocUpdates and Ui.png

* More fixes kill me soon

* - Fix PE Dry Run Issues
- Change Find Command predicate argument types
- Change Find command to find both archieved and checked-in
- Feature to find room by guest name
- Change listCommandUml
- Add findCommandUml
- Portfolio Documentation

* Fixes to merge conflicts

* Fix for Not On FX Application Errors

* Even more unused import changes

* A bit more fixes and coveralls

* Fix for Integer.parseInt for capacity

* omg travis stop

* Update PPP, change gitignore to include config.json to change app name to "Concierge"

* Try fix no newline eof

* Update gitignore to include config.json again

* Edits to devguide and PPP

* Update UserGuide List and Find feature
* Update diagrams

* UserGuide: update undoable commands

* Update grammar
* SampleDataUtil: add yesterday-today booking to r/005

* SampleDataUtil: add expired booking to r/006
* README: update link for ConciergeFinal.png to show properly in index.html and github

* README: show Ui.png

* README: minor edit
* Update PPP links

* Update PPP links

* Update PPP links

* Add instructions for manual testing: autocomplete, find, list

* Change room number for list command
…267) - forced merge

* SampleDataUtil: add yesterday-today booking to r/005

* SampleDataUtil: add expired booking to r/006

* readme fix

* DG: update

* undo readme change
* DG: fix description
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants