19698 Added Account-Id to auth api call + check for null auth info + small cleanup #650
Conversation
| @@ -875,6 +873,8 @@ export default class App extends Mixins(CommonMixin, DateMixin, FilingTemplateMi | |||
| // set the resources | |||
| if (!resources) throw new Error(`Invalid ${this.getEntityType} resources`) | |||
| this.setResources(resources) | |||
| // NB - for some reason, need to call this here so the store updates this getter | |||
| const dummy = this.getFilingData // eslint-disable-line | |||
There was a problem hiding this comment.
This is very strange. Without this here, getFilingData (which returns one of the resource properties) always returned null. I tried a few things, including awaiting next tick, refactoring getFilingData, refactoring a failing App unit test, etc, and this is the only thing that worked consistently.
In any case, this code is safe (won't hurt anything), but I am a tad concerned about other getters that might not work after just setting a value/object in the store. 🤷♂️
| name: item.name, | ||
| legalType: item.legalType | ||
| } as AmalgamatingBusinessIF | ||
| } |
There was a problem hiding this comment.
This item will appear as "not affiliated".
It should never happen that the user was able to add the business but now it doesn't exist at all.
| import { useStore } from '@/store/store' | ||
|
|
||
| setActivePinia(createPinia()) | ||
| const store = useStore() |
There was a problem hiding this comment.
This is how to access the store in a non-Vue class where you can't use the @Getter or @Action decorations/declarations.
PS This is actually type-safe!
|
|
||
| return axios.get(url).then(response => { | ||
| return axios.get(url, config).then(response => { |
There was a problem hiding this comment.
We pass in the account id and the API checks it against the business we're asking about. We'll get a HTTP 403 (Forbidden) back if the business isn't affiliated to this account id.
NB - The API continues to accept "missing account id" and just returns the data if the business is affiliated with any account for the current login (Keycloak token). Ultimately the API should require the account id but if we did that now it might break other things.
severinbeauvais
force-pushed
the
19698
branch
from
294f888
to
c4dfe8b
Compare
- added get after set to update store - added check for empty auth info when re-fetching ting business info - added some safety checks when updating prepopulated data (ie, new holding/primary business) - moved accountId computed to store - added Account-Id header to fetchAuthInfo() - deleted some unused getters/local code - cleaned up some App unit tests
severinbeauvais
force-pushed
the
19698
branch
from
c4dfe8b
to
ce57fb1
Compare
severinbeauvais
requested review from
JazzarKarim,
vysakh-menon-aot,
jamespaologarcia,
ketaki-deodhar and
PaulGarewal
|
/gcbrun |
|
Temporary Url for review: https://business-create-dev--pr-650-dtecq19o.web.app SB says, try this one (as BCREG0020/Severin Dev Account): https://business-create-dev--pr-650-dtecq19o.web.app/amalg-reg-information?id=TaUQpULq2Z&accountid=3040 |
| // safety checks | ||
| if (!business || business.type !== AmlTypes.LEAR) throw new Error('Invalid business') | ||
| if (!business.addresses) throw new Error('Missing business addresses') | ||
| if (!business.authInfo) throw new Error('Missing share classes') |
There was a problem hiding this comment.
This is to display a console error (and however the caller wants to handle the exception) in case there is any error with the data (eg, Riyaz's testing in 19750).
Test resultsAs BCREG0020/Severin Dev Account:
Note the 3 businesses that display as "affiliate to view". Those businesses are either affiliated to a different account under BCREG0020 or to a different login altogether. As a staff user:
Note that all businesses display as affiliated (even though they aren't) because this is a staff (IDIR) login. |
…small cleanup (bcgov#650) * - app version = 5.8.15 - added get after set to update store - added check for empty auth info when re-fetching ting business info - added some safety checks when updating prepopulated data (ie, new holding/primary business) - moved accountId computed to store - added Account-Id header to fetchAuthInfo() - deleted some unused getters/local code - cleaned up some App unit tests * - added some line breaks between code blocks in legal services --------- Co-authored-by: Severin Beauvais <severin.beauvais@gov.bc.ca>
Issue #: bcgov/entity#19698
Description of changes:
- app version = 5.8.15
- added get after set to update store
- added check for empty auth info when re-fetching ting business info
- added some safety checks when updating prepopulated data (ie, new holding/primary business)
- moved accountId computed to store
- added Account-Id header to fetchAuthInfo()
- deleted some unused getters/local code
- cleaned up some App unit tests
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the bcrs-entities-create-ui license (Apache 2.0).