bcgov / namerequest

Public Front End for the Name Request System
1 stars 42 forks source link

#8901 name choice should not have designation added #499

Closed eve-git closed 2 years ago

eve-git commented 2 years ago

Name choice should not have designation added

Issue #: /bcgov/entity#8901

Description of changes:

  1. If a designation has NOT been setup before editing the name, the designation is NOT in the name when no designation input available.
  2. If the designation has been setup before editing the name, the designation goes with the name when no designation input available.
  3. Removed a piece of the code which never reachable.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the namerequest license (Apache 2.0).

severinbeauvais commented 2 years ago

What are your test cases for this change? Please list them in the ticket.

eve-git commented 2 years ago

I just removed designation that added after the first create. In the test case below, the value of name variable is 'TEST ABC INC.'. Please check if the results are expected. test 1.

  1. Create a NR of BC Limited company with 'INC' designation
  2. Edit after payment image.png
severinbeauvais commented 2 years ago

This is not what I asked for. QA does not look at PRs for test notes, so they will not know what you have tested and what they should verify. Please add your test cases to the ticket.

I saw your code change, and I think there may be multiple paths that go through there. You need to test as many of those paths as possible to verify that all cases work as expected. Off the top of my head I can think of the following scenarios:

Add anything else you can think of to this list.

eve-git commented 2 years ago

I did do several test cases for unit test. I had questions for the results, so that I attached the screenshot in this ticket. I wonder if it is what expected?

The code changes are nothing special and only related to the display of the names. The steps of reproduce the error are in the ticket. In the case, should the tester create test cases?

severinbeauvais commented 2 years ago

I did do several test cases for unit test. I had questions for the results, so that I attached the screenshot in this ticket. I wonder if it is what expected?

The code changes are nothing special and only related to the display of the names. The steps of reproduce the error are in the ticket. In the case, should the tester create test cases?

THIS is not the ticket. THIS is the Pull Request.

Yes, the tester is free to create their own test cases. I am looking for evidence that you have run a variety of test cases to verify the fix and that nothing else is impacted.

eve-git commented 2 years ago

OK, I will attach my question to the ticket. Thanks.

eve-git commented 2 years ago

@cameron-freshworks Can you please review the changes? Thanks!