search

cyaoxuan / pe

0 stars 0 forks source link

Verbose error message when applying to a non-organisation #5

Open cyaoxuan opened 1 year ago

cyaoxuan commented 1 year ago

The error message exposes the internals of the programme implementation to the users, which is both confusing and unhelpful to the user. However, the front message "Attempted to apply to a non-organization" is still useful. This was likely due to mishandling of the toString() function or the Messages.format image.png

nus-pe-script commented 12 months ago

Team's Response

The internal implementation can be fixed by unifying the correct user friendly string representation within toString(), which simultaneously fixes the string representation for all cases.

The 'Original' Bug

[The team marked this bug as a duplicate of the following bug]

Success message format not user-friendly

When i enter:

  1. apply Google --title swe this is the success message i get which reveals the internals of the code.

image.png

Added application: {title=swe, stage=resume, status=offered, deadline=12-12-2020, description=None} to seedu.address.model.contact.Organization{name=Google, type=organization, id=google, phone=Optional[91292951], email=Optional.empty, url=Optional[careers.google.com], address=Optional[70 Pasir Panjang Road, #03-71, Mapletree Business City, Singapore 117371], tags=[[bigtech], [competitive], [internship]]}

This is comparable to the more user-friendly success message i receive when using normal add functions

New organization added: police; Id: police-30dee9; Phone: 999; Email: (none); Url: (none); Address: (none); Tags:

image.png

[original: nus-cs2103-AY2324S1/pe-interim#5613] [original labels: type.FeatureFlaw severity.VeryLow]

Their Response to the 'Original' Bug

[This is the team's response to the above 'original' bug]

Agreed.

Though, this is also already covered by our Planned Enhancement: Better Formatting for Contacts.

If a bug matches an enhancement listed in the 'Appendix: Planned Enhancements' of the DG, that bug can be rejected (the tester should not have reported it at all).

You are right that we already have an existing user-friendly representation that was not correctly used in all places. The internal implementation can be fixed in all areas at once by moving the user friendly string representation to be within toString().

Items for the Tester to Verify

:question: Issue duplicate status

Team chose to mark this issue as a duplicate of another issue (as explained in the Team's response above)

Reason for disagreement: [replace this with your explanation]

## :question: Issue response Team chose [`response.Rejected`] - [ ] I disagree **Reason for disagreement:** [replace this with your explanation]