31112 define disabled wait time and change priority wait time to Days#851
31112 define disabled wait time and change priority wait time to Days#851eve-git merged 6 commits intobcgov:mainfrom
Conversation
eve-git
requested review from
flutistar,
mengdong19,
ozamani9gh,
rarmitag,
shaangill025 and
stevenc987
as code owners
|
/gcbrun |
|
Temporary Url for review: https://namerequest-dev--pr-851-xiqzzpzx.web.app |
|
/gcbrun |
|
Temporary Url for review: https://namerequest-dev--pr-851-xiqzzpzx.web.app |
|
/gcbrun |
|
Temporary Url for review: https://namerequest-dev--pr-851-xiqzzpzx.web.app |
|
|
||
| async created (): Promise<void> { | ||
| if ( | ||
| // FF returns 0 either it sets to 0 or the FF is disabled |
There was a problem hiding this comment.
Can you please explain what the different FF values do? And what should be displayed in each case?
| // when flag is 0 or invalid, fall back to stats value | ||
| const statVal = this.getStats?.regular_wait_time | ||
| const statNum = this.normalizeWaitTime(statVal) | ||
| if (statNum > 0) { |
There was a problem hiding this comment.
You might not need all this code.
If line 154 returns undefined (because getStats is null), this comparison still returns the expected value.
Example:
| if (flagVal > 0) return flagVal | ||
| if (flagVal < 0) return '-' | ||
|
|
||
| // when flag is 0 or invalid, fall back to stats value |
There was a problem hiding this comment.
How could the FF be invalid? What value would that be?
There was a problem hiding this comment.
it will be 0 if FF is invalid or disabled
There was a problem hiding this comment.
Oh, do you mean if the FF is >0 or <0?
There was a problem hiding this comment.
No, ignore my comment.
How can be FF be invalid?
There was a problem hiding this comment.
The FF can never be invalid. It will always be a number.
If you change this comment to "when FF is 0, fall back to stats value", I think it makes sense.
There was a problem hiding this comment.
I removed the comment. The comment related to FF is at line 115-119
|
/gcbrun |
|
Temporary Url for review: https://namerequest-dev--pr-851-xiqzzpzx.web.app |
|
/gcbrun |
|
Temporary Url for review: https://namerequest-dev--pr-851-xiqzzpzx.web.app |
| /* | ||
| * Feature-flag semantics for hardcoded_*_wait_time: | ||
| * - flag > 0 : use the flag value (explicit positive wait time). | ||
| * - flag === 0 OR FF disabled: fall back to backend stats. |
There was a problem hiding this comment.
If the FF is "disabled", it's still providing a value.
I think it makes sense for the "disabled" value to be 0. It's like that right now:
| * Feature-flag semantics for hardcoded_*_wait_time: | ||
| * - flag > 0 : use the flag value (explicit positive wait time). | ||
| * - flag === 0 OR FF disabled: fall back to backend stats. | ||
| * - flag < 0 : flag indicates the wait time is disabled. |
There was a problem hiding this comment.
Using the word "disabled" here again is confusing.
How about saying "flag indicates API value should be used"?
| return statVal | ||
| } | ||
|
|
||
| return '-' |
There was a problem hiding this comment.
Yeah, this is simple and effective now 👍
There was a problem hiding this comment.
Question: do we ever want to display "0" on the page? The current code doesn't support that.
|
|
||
| get showRegularTooltip (): boolean { | ||
| const val = Number(this.regularWaitTime) | ||
| return Number.isFinite(val) && val > 0 |
There was a problem hiding this comment.
I think you don't need all the checks here... This works:
But, if you want to keep this, I'm OK with it.
|
/gcbrun |
severinbeauvais
left a comment
severinbeauvais
left a comment
There was a problem hiding this comment.
This code seems to work but I still find it a bit confusing.
I approve this change. Up to you if you want to clean anything up.
|
Temporary Url for review: https://namerequest-dev--pr-851-xiqzzpzx.web.app |
* 30381 cleaned up some env keys and fixed breadcrumbs (#843) * - deleted obsolete key VUE_APP_BUSINESSES_URL - app = 5.5.46 - fixed Business Registry URL in Affiliation Error Modal - fixed Staff Dashboard breadcrumb * - updated browserslist --------- Co-authored-by: Severin Beauvais <severin.beauvais@gov.bc.ca> * 30656 Updated LaunchDarkly module to support orgs (#844) * - app version = 5.6.0 - imported latest LD JS client SDK v3.9.0 - updated UpdateUserMixin to create contexts - updated featureFlags module to use contexts * - renamed updateUser -> updateLaunchDarkly - used getter instead of computing KC roles again - combined updateUser + updateLaunchDarkly - renamed featureFlags.ts -> launchDarkly.ts - misc cleanup --------- Co-authored-by: Severin Beauvais <severin.beauvais@gov.bc.ca> * 30554 DBA NR magic link (#846) Co-authored-by: bobby <bobbyjin935@gmail.com> * 29274 Upgraded to Vite (#845) * - misc cleanup - updated babel config file - moved index.html from /public to / - updated index.html - deleted obsolete jest config file - app version = 5.7.0 - updated scripts in package file - deleted obsolete dependencies - added necessary dependencies - recreated pnpm lock file - updated patch to use import.meta - deleted obsolete postcss config file - added robots file - replaced process.env -> import.meta.env - replaced JEST_WORKER_ID -> VITEST - added woff2 fonts - updated font sources in CSS - replaced getBaseUrl with direct sessionstorage.getItem() - fixed some image sources - deleted obsolete payment-utils.ts - changed getConfig to non-async - added vuetify css import - changed v-img to img - deleted unused isJestRunning computed - replaced appBaseURL -> namexApiUrl - added check for valid user/org before callling UpdateLdUser - added BASE_URL to session storage - added NAMEX_API_URL to session storage - updated Vuetify import - deleted appBaseURL export - deleted commented-out console.log - commented out unneeded console.log (for now) - updated unit tests - updated types in tsconfig file - added vite config file - updated vue config file * - fixed v-icon size and color --------- Co-authored-by: Severin Beauvais <severin.beauvais@gov.bc.ca> * Modify README for pnpm version and commands Update pnpm installation and development commands * 30987 Upgraded to Pinia (#847) * - app version = 5.8.0 - installed Pinia dependencies - updated Getter and Action decorators - misc cleanup - created Filing Types enum - created Error interface - created Payment models - removed unneeded await for store actions - converted Vuex Modules to separate stores - created getStores plugin - cleaned up error store - cleaned up payment store - delete env config - deleted obsolete properties from main store state - deleted unneeded stateModel property - refactored actions and mutations into "sub store" - refactored getters into "sub store" - refactored state into "sub store" - created new Pinia main store from sub stores - now call mutations as functions instead of "commit" - misc cleanup / type fixes - moved getXXX in main.ts - attached Vuex store to Vue instance - mocked Vuex store for unit tests - updated misc comments - final commit with store as 3 sub-stores * - changed actions back to exported functions - changed getters back to exported functions - changed state back to exported const - updated store definition - misc fixes and cleanup - updated Reg Search API URL in example env * - changed getNrState to return NrState - cleaned up some typings - changed getNrId to return number * - attached Vuetify to Vue - use current Vuetify instance to get breakpoint * - small vault comment fixes - approve building "vue-demi" to solve "missing export" issue * - fixed "cannot read null key properties" * - updated lockfile - updated pnpm lock file - lock file with pnpm v9 * - added exclude in Vite config --------- Co-authored-by: Severin Beauvais <severin.beauvais@gov.bc.ca> * 31112 define disabled wait time and change priority wait time to Days (#851) * define disabled wait time and change priority wait time to Days * update * lint error * Update * linting * update * 30032 - NR Payments - Renewal Payments do not have option to add Folio Number (#853) * bugfix: Add folio number to RENEW payment when the first payment already had it - bump version to 5.8.3 * chore: Update folio number assignment logic to use PaymentAction.CREATE filter * cleanup * 29381 - xpro name allowed length increased from 40 to 60 (#852) * Update `MRAS_MAX_LENGTH` constant to 60 in new-request constants * version bump * chore: bump version to 5.8.3 * Add new FF for priority wait time tooltip (#855) * 2551 update priority tooltips (#856) * update priority tooltips * Remove unnecessary newline in stats.vue * 29381 - rules and validation updates in name field (#857) * fix: name input rules logic and add xpro validation - add validation/disable button in sub search modal * fix: correct defaultRules return order in name input component * fix: update search button logic and add xpro name validation - Update `isSearchBtnDisabled` logic for button disablement - Add `isValidXproName` getter for XPRO name length validation * chore: bump version to 5.8.6 and remove unused import in search component * fix: adjust max length validation for XPRO name flow - Use dynamic max length based on `isXproFlow` condition. * 31568 - remove Release Notes link from the footer (#858) * Update `MRAS_MAX_LENGTH` constant to 60 in new-request constants * version bump * chore: bump version to 5.8.3 * fix: remove "Release Notes" link from footer and update sbc-common-components patch * build lock with pnpm 9 --------- Co-authored-by: Séverin Beauvais <severinbeauvais@users.noreply.github.com> Co-authored-by: Severin Beauvais <severin.beauvais@gov.bc.ca> Co-authored-by: bobdev-94 <jinbobby677@gmail.com> Co-authored-by: bobby <bobbyjin935@gmail.com> Co-authored-by: eve-git <54647458+eve-git@users.noreply.github.com>
3 participants
Issue #: /bcgov/entity###
bcgov/entity#31112
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).