RUN-2032: Percentage in number showing in progress bar instead of undefined% #8974
Conversation
There was a problem hiding this comment.
One heads-up, one common habit that engineers develop is to validate solutions before implementing them to avoid missing important aspects.
The architecture meeting on Mondays is an example of folks validating their ideas.
Whenever starting to work on a task that doesn't have steps regarding how to solve it, l'd like you to investigate the issue and send me or other folks a message with the plan of how this issue will be solved.
In this type of communication, it is also helpful to point out how you concluded that what you're solving is the problem in the first place.
This will help us to ensure you're on the right track, and that you have all the info you need, so that once you execute your plan, the PR is ready for review.
Currently, this will be moved back to in progress, and we can discuss it more tomorrow.
| @@ -288,29 +288,31 @@ | |||
| class="dateStarted date" | |||
| colspan="2" | |||
| > | |||
| <!--getting vue warning: Invalid prop: type check failed for prop "modelValue".Converted :value to :modelValue--> | |||
There was a problem hiding this comment.
A few things here:
-
good job correctly interpreting the vue warning;
-
whenever possible, it's good to avoid adding comments to the template as they get rendered in the dev tools;
-
the gist of adding comments is to help to clarify why something was done in a specific way, that may not be obvious by just reading the code. Therefore it is good to think who the audience is, and its relevance - including if there is any other mechanism that could alert/prevent this issue from happening.
In this case, vue will emit warnings that will help the team avoid making this mistake, which means, there isn't much value registering as a comment that value isn't an acceptable prop anymore.
As for the audience, please let me know if I'm wrong, but I believe that you meant to leave this comment for the PR reviewer. Whenever that's the case, please use the github functionality to comment in the PR itself.
- therefore, please ✂️ this comment
| striped | ||
| type="default" | ||
| label | ||
| :label-text=" | ||
| $t('job.execution.starting.0', [ | ||
| runningStartedDisplay(exec.dateStarted.date), | ||
| // if exec.dateStarted is null or undefined,it doesn't throw an error but instead returns undefined | ||
| runningStartedDisplay(exec.dateStarted?.date) || 'N/A', |
There was a problem hiding this comment.
Question, what led you to the conclusion that this change was needed? Have you seen a scenario where this happens?
There was a problem hiding this comment.
Question, what led you to the conclusion that this change was needed? Have you seen a scenario where this happens?
I was going through line by line code between progress bar (first I saw in vue dev tool to confirm which line of code I have to see to make it right) and started understanding the functions. So the runningStartedDispaly requires the valid date to return the right output. I did some changes and still it didn't work. Then I applied ternary operator(with the help of few google search) to debug the code.
There was a problem hiding this comment.
Question, what led you to the conclusion that this change was needed? Have you seen a scenario where this happens?
Ok so I fixed it now. It was just unnecessary change from my side. Ignore it
| const now = moment(); | ||
| const startDate = moment(exec.dateStarted.date); | ||
| const diff = now.diff(startDate); | ||
| const percentage = Math.floor((diff / exec.job.averageDuration) * 100); |
There was a problem hiding this comment.
One heads-up, if it's the first time running a job, averageDuration isn't available. We shouldn't be showing 0 in this case.
| @@ -801,6 +802,25 @@ export default defineComponent({ | |||
| } | |||
| return 0; | |||
| }, | |||
|
|
|||
| // Below code makes the progress bar reach closer to 100% | |||
fdevans
changed the title
Is this a bugfix, or an enhancement? Please describe.
"undefined%" message appears in the progress bar instead of percentage. Fixed it to make it shows the percentage in number.
Describe the solution you've implemented
Describe alternatives you've considered
Additional context