Skip to content

[Issue #8399 / #8605] Add email processor for workflow management#8990

Open
chouinar wants to merge 9 commits intomainfrom
chouinar/8399-email-processor
Open

[Issue #8399 / #8605] Add email processor for workflow management#8990
chouinar wants to merge 9 commits intomainfrom
chouinar/8399-email-processor

Conversation

@chouinar
Copy link
Collaborator

Summary

Fixes #8399 + #8605

Changes proposed

  • Added logic to workflows for sending emails when entering a workflow state that requires approvals

Context for reviewers

We want to send approval emails only when first entering an approval state. We know this by checking:

  • Is the state we're entering an approval state - this was determined by flipping the configuration for approvals to be state->approval config (calculated from the existing config)
  • Is the state we're entering the same as the prior state - if so, we're already in that state, we don't want to send emails again

The email logic is mostly a lot of error checks, logging and guard rails. The actual email contents aren't 100% finalized, but are good enough for a prototype and testing.

Validation steps

See tests - still not able to run service on its own. Soon though.

Comment on lines +118 to +120

def on_enter(self, state_machine_event: StateMachineEvent, event_data: EventData) -> None:
print(event_data)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this supposed to do anything else or is it a stub for future use?

Comment on lines +50 to +51
except Exception:
logger.exception("Failed to send email for workflow", extra=log_extra)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know we're not fully up and running yet, but are we ok with this failing silently?

Comment on lines +57 to +59
# If the state machine's state is changing as part of this
# then we don't want to do anything. We only want to send
# emails when first entering the state.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# If the state machine's state is changing as part of this
# then we don't want to do anything. We only want to send
# emails when first entering the state.
# If the state machine's state is NOT changing as part of this
# then we don't want to do anything. We only want to send
# emails when first entering the state.

Comment on lines +37 to +38
for approval_config in self.approval_mapping.values():
self.state_approval_mapping[approval_config.approval_state] = approval_config
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can two different events map to the same approval_state (or will it be possible in the future)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create the common logic for sending emails for workflow management, including specific logic for approvals

2 participants