Harithhh06 / pe

0 stars 0 forks source link

delete_session not optimised #13

Open Harithhh06 opened 1 week ago

Harithhh06 commented 1 week ago

Screenshot 2024-11-15 at 5.03.59 PM.png

Screenshot 2024-11-15 at 5.05.22 PM.png

Description

deleting sessions is not optimised. In a case where a session is input for more than 20 users, this may take a very long time to input all member indexes. Even worse if the indices are far apart eg. m/1 m/24 m/36 m/123. This makes it very unoptimised even for fast typers.

nus-pe-bot commented 4 days ago

Team's Response

This limitation is inherent to the CLI design, as it requires individual specification for operations like deletion.

Even in enterprise software like Git, individual branches must be explicitly specified for deletion. While implementing an option to delete all sessions might seem like an alternative, it carries significant risks and may not align with your use case.

For cases where indices are far apart, it might be more efficient to use the find_session command to locate the specific session first and then delete it afterward.

Items for the Tester to Verify

:question: Issue response

Team chose [response.NotInScope]

Reason for disagreement: I understand where you are coming from but I think the comparison of deleting a session and deleting a git branch is not very relevant as the implications of deleting a session are much lower and is based on how it was implemented by the team. It seems that a session is added as a field to a member instead of an object that exists outside of a member. Though I am not disagreeing with your implementation, I feel this might be counterintuitive to other features of your app being:

More often than not, if there was mistake in the session it would make sense to delete for all members. Therefore, a user would have to delete a session and re-add it back, typing all the member indexes multiple times which may even potentially cause more mistakes. Since it falls under a feature not being fully complete I feel that this warrants a FeatureFlaw.

However I do agree with keeping the current implementation to delete sessions for individual members would still have its place in the event of misinput for individual members.


## :question: Issue severity Team chose [`severity.Low`] Originally [`severity.Medium`] - [x] I disagree **Reason for disagreement:** I think with the cases I mentioned above, the inconvenience may occur more frequently than a rare instance, becoming an occasional inconvenience. Even more so for CCA leaders that manage larger CCAs as mentioned before, this issue may be an even bigger inconvenience. Case: * User inputs `add_session s/training 11 d/15 Nov 2024 m/1 m/1 ... m/20` for all cca members * User realizes mistake in training number or date * User deletes session `delete_session s/training 11 d/15 Nov 2024 m/1 m/1 ... m/20` but misses a few members with index in between * User adds new correct session for all members * Members with wrong session still remain Therefore I feel it is apt that it remains a `severity.Medium` at least.