[CST-1368] Change ECTs induction start date and cohort when DQT record updated #3265
Conversation
|
Created review app at https://ecf-review-pr-3265.london.cloudapps.digital |
|
Smoke tests passed against the review app. |
lazydevorg
changed the title
lazydevorg
requested review from
peteryates,
cwrw and
tonyheadford
as code owners
| def call | ||
| return false unless FeatureFlag.active?(:cohortless_dashboard) | ||
| return false if @dqt_induction_start_date.nil? | ||
| return false if @participant_profile.induction_start_date.present? |
There was a problem hiding this comment.
I think we still should go ahead even when the participant has already an induction_start_date instead of returning false.
The reason is that some of their induction records might still be in the wrong cohort and need to be amended.
| dqt_cohort = Cohort.containing_date(@dqt_induction_start_date) | ||
| return false if dqt_cohort.nil? | ||
|
|
||
| if dqt_cohort == participant_cohort |
There was a problem hiding this comment.
Same as above. We should still call amend_cohort in this situation because even if the latest induction record is in the right cohort, some of the old induction records might still be in the wrong cohort and need to get it amended.
There was a problem hiding this comment.
if we do this and, as per your comment above, then this job will run all the time for each participant.
I think the ask was to do it just for the participants who did not have the induction start date set.
I'll double check whether this is still the case.
There was a problem hiding this comment.
Yes, exactly, because:
- we already have participants with wrong cohorts in their historical induction records and this is the tool to fix them, no matter their existing induction start date.
- their existing induction date might not match the one in their DQT record and better to have the right one coming from DQT.
- we want to run this tool for ALL participants only one time to update their induction start date
There was a problem hiding this comment.
Once this one-off task is done, then your tool will only run for each new participant, for re-validations in support and the periodic job programmed for participants with no QTS
| current_induction_record = @participant_profile.induction_records.latest | ||
| participant_cohort = current_induction_record.cohort | ||
| dqt_cohort = Cohort.containing_date(@dqt_induction_start_date) | ||
| return false if dqt_cohort.nil? |
There was a problem hiding this comment.
I think that in this case we should also save an error for the participant to track that the target cohort for this participant has not been setup yet in our service.
This might help to warn us that we need to setup missing cohorts.
|
|
||
| def save_error_message(amend_cohort) | ||
| error = SyncDqtInductionStartDateError.find_or_create_by(participant_profile: @participant_profile) # rubocop:disable Rails/SaveBang | ||
| error.error_message = amend_cohort.errors.full_messages.join(", ") |
There was a problem hiding this comment.
Please, see my comment below about each error recorded in its own entry.
ltello
force-pushed
the
CST-1368-change-ects-cohort-when-dqt-updated-2
branch
5 times, most recently
from
b1e6af7
to
04c872b
Compare
|
@lazydevorg - this looks good to me, will approve once the conflict is resolved 👌🏽 |
ltello
force-pushed
the
CST-1368-change-ects-cohort-when-dqt-updated-2
branch
2 times, most recently
from
b6f0246
to
55d7d0a
Compare
ltello
force-pushed
the
CST-1368-change-ects-cohort-when-dqt-updated-2
branch
from
55d7d0a
to
55a4be4
Compare
Context
Changes proposed in this pull request
Participants::SyncDqtInductionStartDateadded and called fromParticipants::ParticipantValidationForm