search

AY1920S1-CS2103-F09-4 / main

Insurelytics
https://ay1920s1-cs2103-f09-4.github.io/main/
MIT License
1 stars 5 forks source link

Bug in eligiblepeople and addtag #220

Closed ybchen97 closed 4 years ago

ybchen97 commented 4 years ago

Describe the bug addtag runs into invalid person index after using eligiblepeople on a policy that has no eligible persons.

To Reproduce Steps to reproduce the behavior:

  1. listpolicy
  2. eligiblepolicies 1 (a policy that has no eligible persons)
  3. listpolicy
  4. addtag 1 t/hello

Actual behavior Runs into error "The person index provided is invalid." I suspect what happens is that when I call eligiblepeople, this sets the "persons list" to the list of eligible persons. When I try to navigate to the policy list to find out and add the tags that I'm missing, it throws an invalid index error because "persons list" is still the list of people who are eligible for the policies, which is currently an empty list.

Expected behavior I think the problem lies in the question of "which list am I referring to?" In this context, when I see eligiblepeople return an empty list, I would navigate to the policy page and try to add the tags that I am missing. Then, I would expect to be able to add a tag to the person in ''last shown list", which from the user's perspective, is the list that was shown before the empty list.

larrylawl commented 4 years ago

We fixed this issue temporarily by explaining in the tutorial of the user guide.

olivercheok20 commented 4 years ago

I feel like this is valid behaviour though, especially since we illustrated it in the user guide. Like if for instance a user does a search and filters the people list and then switches to listpolicy, any (person) index would be referring to the new filtered list.

If we wanted to change this behaviour though, we could just update the model with PREDICATE_SHOW_ALL_PERSONS when we do any command that changes the lefthand panel to view policies when the people list is empty, and vice versa for policies. Let me know what you guys think haha I think it should be a quick fix. @ybchen97 @larrylawl

olivercheok20 commented 4 years ago

In the solution I suggested above, the expected behaviour would be -> if a person search comes up empty and then the user changes to view policies and tries to refer to a person index, that index would refer to the global list of people.

larrylawl commented 4 years ago

Yup i do agree that this is a valid behaviour; jus needa explain in the tutorial which we did. lets keep to this then.