search

bcgov / namerequest

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

17727 Refactored restoration flow #694

Closed JazzarKarim closed 1 year ago

JazzarKarim commented 1 year ago

Issue #: /bcgov/entity#17727

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).

JazzarKarim commented 1 year ago

/gcbrun

bcregistry-sre commented 1 year ago

Temporary Url for review: https://namerequest-dev--pr-694-1jkkd4vv.web.app

JazzarKarim commented 1 year ago

/gcbrun

bcregistry-sre commented 1 year ago

Temporary Url for review: https://namerequest-dev--pr-694-1jkkd4vv.web.app

JazzarKarim commented 1 year ago

@ketaki-deodhar @leodube-aot OK to merge?

ketaki-deodhar commented 1 year ago

@JazzarKarim

  1. selected XPRO business and selected Jurisdiction 'Federal'
  2. clear business by clicking 'X'
  3. choose another XPRO business from business lookup
  4. Jurisdiction is pre-selected and is same as was previously selected

First selected this business image

Second selected business image

leodube-aot commented 1 year ago

Hey Karim, not sure if this is a bug caused by the rebase but I noticed a difference between this rebase and dev. This occurs with BC0871294 and then selecting "Numbered Company".

restore bug

JazzarKarim commented 1 year ago

Hey Karim, not sure if this is a bug caused by the rebase but I noticed a difference between this rebase and dev. This occurs with BC0871294 and then selecting "Numbered Company".

restore bug

Yup, this is exactly the bug that I mentioned in teams. Thanks a lot Leo! Working on the fix now.

leodube-aot commented 1 year ago

@JazzarKarim

  1. selected XPRO business and selected Jurisdiction 'Federal'
  2. clear business by clicking 'X'
  3. choose another XPRO business from business lookup
  4. Jurisdiction is pre-selected and is same as was previously selected ...

I was just working on a similar bug prior to this refactor (/bcgov/entity#17630). You'll probably just want to clear the jurisdiction values using setters in onBusiness in business-lookup-fetch.vue

JazzarKarim commented 1 year ago

/gcbrun

JazzarKarim commented 1 year ago

@JazzarKarim

  1. selected XPRO business and selected Jurisdiction 'Federal'
  2. clear business by clicking 'X'
  3. choose another XPRO business from business lookup
  4. Jurisdiction is pre-selected and is same as was previously selected ...

I was just working on a similar bug prior to this refactor (/bcgov/entity#17630). You'll probably just want to clear the jurisdiction values using setters in onBusiness in business-lookup-fetch.vue

Thank you so so much Leo. The issue was actually with the conditional that's in the NameInput template. It should be fine now!

bcregistry-sre commented 1 year ago

Temporary Url for review: https://namerequest-dev--pr-694-1jkkd4vv.web.app

leodube-aot commented 1 year ago

@JazzarKarim

  1. selected XPRO business and selected Jurisdiction 'Federal'
  2. clear business by clicking 'X'
  3. choose another XPRO business from business lookup
  4. Jurisdiction is pre-selected and is same as was previously selected ...

I was just working on a similar bug prior to this refactor (/bcgov/entity#17630). You'll probably just want to clear the jurisdiction values using setters in onBusiness in business-lookup-fetch.vue

Thank you so so much Leo. The issue was actually with the conditional that's in the NameInput template. It should be fine now!

Hey Karim, the latest commit fixes the bug I pointed out and it looks great :)! My comment was for Ketaki's bug, about the Jurisdiction select retaining state when the business changes. I am still able to replicate the bug in the latest temo url.

Also I just realized it's technically a bug in dev too lol

severinbeauvais commented 1 year ago

Karim is out of the office today and this PR looks good to me, and we want to move the fetaure branch to Dev asap. If there are latent bugs then we can fix them in main branch. So I'm merging this.