okg00dbye / pe

0 stars 0 forks source link

review commands will lead to only one book being displayed #11

Open okg00dbye opened 3 years ago

okg00dbye commented 3 years ago

e.g

  1. searchReview n/the (a few books will be displayed)
  2. deleteReview 1 rn/1 or editReview or addReview (now only the edited book will be shown, and the user cannot edit the reviews of e.g index 2 even though they wanted to previously)

this makes it quite hard for the user to edit/delete/add reviews to multiple books at the same time, because they will always need to call list again, then call searchReview again to see the other books they want to edit

nus-se-bot commented 3 years ago

Team's Response

Hello, thanks for noticing this. Appreciate your effort and time spent in trying our product, as well as the suggestions that you gave. Our team understood that you may have certain doubts about the current product design. However, after careful consideration, our team has decided to reject this issue.

  1. Consider this scenario. The user uses list to list all the books, and without using searchReview the user uses addReview to add a new review to a book directly. This is actually quite common since sometimes librarian may just want to record a new review from survey without bothering to see the existing review list of a book. In this case, it should not be reasonable to display all the other books after adding the review. Firstly, it is unreasonable to just display all the reviews of all the books, since the intention of the librarian is just to add a review to a particular book. Secondly it is unreasonable to show all the books but only display the review list of the affected book. This will cause great inconsistency in the user interface. Note that it is possible to add a review to a book just using list without using searchReview.

  2. For deleteReview and editReview, it is recommended to use a searchReview before deleting or editing. In this case, the reason for the design is to help users to focus on the changes that has been done to a certain book. If all the books are displayed after editing or deleting the review, the user may easily be lost about what changes have been made previously, which could cause certain problem since there is no redo or undo implementation in the application.

  3. In any case, the design can help users to focus on what changes have been done to which book, so that they can know what to do if they accidentally execute an undesired command containing typo or other issues.

  4. There is actually no command that allows user to edit, delete, or add reivew to multiple books concurrently. Only 1 review of 1 book can be affected in the execution of a command. This is because the review is recorded on a person by person basis, and it is not very common that different readers will give exactly the same review with the same rating to different books, even if this may happen sometimes.

  5. There is no need to use both list and searchReview when deleting or editing a review, and just using searchReview will already enables the user to see reviews to delete or edit.

  6. The Developer Guide also explicitly mentions that showing only the affected book is the desired behavior.

image.png

Additionally, our team has also decided to change the severity because the advantage of the design outweigh its disadvantage as is mentioned above.

Given the reasons above, our team has decided to reject the issue. Thank you again for your response, and our team hopes that you can understand our decision.

Items for the Tester to Verify

:question: Issue response

Team chose [response.Rejected]

Reason for disagreement: Hi, thank you for your reply! :-)

For 1, I understand that 'searchReview' need not always be called before adding a review, however this addresses a different issue. The main issue is that the change to display only one book after calling a review command makes it so that one cannot call another review command (on another book) right after that.

Consider the scenario that the librarian want to add two reviews, one to the Pride and Prejudice at index 1, and one to A brief history of time at index 2

image.png

The librarian will enter the commands: "addReview 1 ra/4 re/test" and this will bring them to this UI

image.png

Now, consider the libriarian does not remember the index of the other book they wanted to add the review too, hence they will call "list" again, and after that the command they actually wanted to do which is "addReview 2 ra/3 re/test". If the librarian does remember that the other books they wanted to add a review to is at index 2, they might try calling "addReview 2 ra/3 re/test" , but this will lead to an invalid index error, which can be replicated by following these steps. What this means is that the librarian will need to call 'list' at minimum after every time they call 'addReview' in order to add review to different books, and is the main issue for point 1 here, which is a need to call 'addReview' -> 'list' -> 'addReview' -> 'list' -> and so on.. as opposed to 'list' -> 'addReview' -> 'addReview' -> ...

  1. Understand that it is recommended that call searchReview, however the issue here is how many times you need to call searchReview. If you want to edit reviews of multiple books, ideally the behaviour should be smth like 'searchReview', then multiple edit commands such as 'edit 1 re/ ra/', 'edit 3 re/ ra/' etc. however this is currently impossible because there will only be one book after the first edit command. Hence, the current implementation will follow smth more like 'search' -> 'edit' -> 'search' again -> 'edit' -> 'search' again-> and etc.

  2. Restricting the list to only one book may not be the best way to achieve this effect, e.g using UI to highlight the user's changes etc. In which case, the disadvantages of restricting the list to only one book (big increase in the number of commands the user needs to enter) is arguably greater than the advantages

  3. Understand that there is no concurrent command, however the issue is with not being able to enter/chain review commands one after another, not with a lack of concurrent command.

  4. searchReview currently supports only ISBN and name. As it is impossible to remember the 13 digit ISBN from memory, a user who wants to searchReview by ISBN will most definitely have to call list first to refer to their desired book's ISBN. Directly searching review by name is possible, however as memory is not perfect either, having a small misremembering or typo of the name can typically cause the user to call list again to find the correct name they want. However even if its not always 'list' -> 'searchReview', it doesn't take away that solely 'search' -> 'edit' -> 'search' again -> 'edit' still requires the user to enter extra commands.

Apologies for any miscommunication in my bug report, but I think this can be summarised with the idea that there is a lot of unnecessary need to call 'searchReview' or at the minimum call 'list' every time you want to CRUD reviews of a different book, which can definitely pose as an inconvenience that accumulates considerably over time, and hence should be accepted as a feature flaw.


:question: Issue severity

Team chose [severity.VeryLow] Originally [severity.Medium]

Reason for disagreement: image.png

should not be very low as it is not purely cosmetic should not be low because search/add/edit reviews is part of the normal operations of the product