16636 Show designation for amalgamation NRs #660
Conversation
- now show designation for amalgamation NR - added a code comment to help future devs
severinbeauvais
requested review from
chenhongjing,
JazzarKarim and
ketaki-deodhar
| @@ -401,7 +401,6 @@ export default class Search extends Mixins(CommonMixin, NrAffiliationMixin) { | |||
| } | |||
|
|
|||
| get showDesignation (): boolean { | |||
| if (this.isAmalgamation) return false | |||
There was a problem hiding this comment.
This hid the designation for amalgamation NRs, but startAnalyzeName() expected a designation, so the user was blocked after they clicked "Check this Name". And the error was not displayed in the designation component because it was not rendered!
I know the code I've deleted looks intentional but there's no comment telling me about this business rule so I assume it's incorrect. (Especially since it blocked named amalgamation NRs.)
There was a problem hiding this comment.
Sounds good. Just for my understanding, Amalgamate shows designation today (in DEV). Is this the correct screen? Just trying to understand if I am looking at right screens :)
There was a problem hiding this comment.
Yes, that is the correct screen. Thanks for comparing with NR UI in Dev -- makes me feel better about this change because I haven't received a lot of straight answers today :)
|
/gcbrun |
|
Temporary Url for review: https://namerequest-dev--pr-660-h1k6oax7.web.app |
severinbeauvais
merged commit 511f7bd
into
bcgov:feature-way-of-navigating
- now show designation for amalgamation NR - added a code comment to help future devs
- now show designation for amalgamation NR - added a code comment to help future devs
- now show designation for amalgamation NR - added a code comment to help future devs
Issue #: bcgov/entity#16636
Description of changes:
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).