Skip to content

[MS-790] Add follow-up steps to session steps instead of replacing them#970

Merged
BurningAXE merged 1 commit into
mainfrom
enrol-last-after-confirm
Nov 21, 2024
Merged

[MS-790] Add follow-up steps to session steps instead of replacing them#970
BurningAXE merged 1 commit into
mainfrom
enrol-last-after-confirm

Conversation

@BurningAXE
Copy link
Copy Markdown
Contributor

Previously each action (including follow-ups) replaced old steps (in the Orchestrator and consequently in the OrchestratorCache) with its own steps. This lead to the issue where the following actions:

  1. Identify
  2. Confirm Identity
  3. Enrol Last
    lead to Enrol Last failing because when Enrol Last looks in the Cache it only finds the step(s) for Confirm Identity.

With this fix each follow-up only adds its steps to the ones already in the cache. This way all session steps are available regardless of how many follow-ups have happened. The Cache is cleared when a new session is created so this change should not lead to mixing of steps between sessions. That said - can you find any edge cases where this may backfire?

@BurningAXE BurningAXE requested review from a team, TristramN, alex-vt, alexandr-simprints, luhmirin-s, meladRaouf and ybourgery and removed request for a team November 11, 2024 11:25
@cla-bot cla-bot Bot added the ... label Nov 11, 2024
@BurningAXE BurningAXE force-pushed the enrol-last-after-confirm branch from 59cfaa3 to 18d6329 Compare November 11, 2024 12:12
Copy link
Copy Markdown
Contributor

@alex-vt alex-vt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just in case a check: is this fix supposed to be in release/2024.2.1, not in main?

@BurningAXE
Copy link
Copy Markdown
Contributor Author

Just in case a check: is this fix supposed to be in release/2024.2.1, not in main?

We'll decide tomorrow but there are 3 options:

  1. Release in 2024.2.1 - best case scenario if considered safe
  2. Have it ready for a 2024.2.2 hotfix if we don't include in 2024.2.1 but turns out to be an issue for a project
  3. Just merge it in main and release in next major release (2025.1.0 I guess)

@BurningAXE BurningAXE force-pushed the enrol-last-after-confirm branch from 18d6329 to 3f3b99e Compare November 20, 2024 18:00
@BurningAXE BurningAXE changed the base branch from release/2024.2.1 to main November 21, 2024 10:52
@BurningAXE BurningAXE force-pushed the enrol-last-after-confirm branch from 3f3b99e to 2e792f6 Compare November 21, 2024 10:59
@sonarqubecloud
Copy link
Copy Markdown

@BurningAXE BurningAXE merged commit df9ab42 into main Nov 21, 2024
@luhmirin-s luhmirin-s deleted the enrol-last-after-confirm branch February 17, 2025 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants