adds polling on prediction run with UI progress notification#3
Conversation
officialid130-13e13
left a comment
officialid130-13e13
left a comment
There was a problem hiding this comment.
Overall the changes look good. The only thing I noticed is that enablePrediction is hardcoded to true in task-definition-effort.component.ts (around line 22). Since we now have a unit‑level flag - unit.allowEffortPredictions, this value should come from the unit rather than being hardcoded.
Fair point, I think I just decided to leave this as I wasn't sure whether we still wanted to have task level control over running predictions, we should discuss again as a team in the weekly meeting to see what we want to do and either delete this checkbox or update to include your suggestion here so we aren't doubling up on work |
We do want to keep task level control over whether predictions are enabled. Each task gets its own prediction job and value so even if we kick it off as a batch job from the unit, we need to keep the ability for the unit staff to disable and enter manually. This discussion is outside of the scope of this pull request. I think the changes made are good. |
|
|
||
| @Injectable() | ||
| export class SidekiqJobService extends CachedEntityService<SidekiqJob> { | ||
| protected readonly endpointFormat = 'sidekiq/:id:'; |
There was a problem hiding this comment.
Revert this change, I have found that it breaks the integration for retrieving Sidekiq jobs on the frontend, meaning that it now only ever shows "prediction failed".
| if (this.pollSub) { | ||
| this.pollSub.unsubscribe(); | ||
| } | ||
| } |
There was a problem hiding this comment.
I think it would also be wise to implement an ngOnChanges(changes: SimpleChanges) as currently when switching between tasks the html for a prediction succeeding or failing is keeping the phantom state of the last task.
private resetPredictionState() {
this.isPredicting = false;
this.predictionStatus = undefined;
this.jobId = null;
}
ngOnChanges(changes: SimpleChanges) {
if (changes['taskDefinition']) {
this.resetPredictionState();
}
}Also ensure to import { OnChanges, SimpleChanges } from '@angular/core';
4 participants
Description
Polls backend for job status and updates UI accordingly
Type of change
How Has This Been Tested?
Manual testing in browser
Testing Checklist:
Checklist: