Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stacked PR's of - Workflow override #4773

Merged
merged 34 commits into from
Nov 16, 2023
Merged

Stacked PR's of - Workflow override #4773

merged 34 commits into from
Nov 16, 2023

Conversation

djabarovgeorge
Copy link
Contributor

What change does this PR introduce?

Why was this change needed?

Other information (Screenshots)

djabarovgeorge and others added 26 commits October 30, 2023 12:10
…dal-repository' into NV-3117-create-workflowoverride-dal-repository
…dal-repository

Create Workflow Override Repository
…ride-use-case

Create workflow override use-case
…ride-use-case

Update workflow override use-case
…e-use-case

Get workflow override use-case
…ride-use-case

Delete workflow override use-case
…es-use-case

Get workflow overrides use case
…he-trigger

Apply Workflow Overrides during the trigger
# Conflicts:
#	apps/api/src/app/events/usecases/parse-event-request/parse-event-request.usecase.ts
#	pnpm-lock.yaml
_workflowId: 1,
_environmentId: 1,
},
{ unique: true }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Create it as unique so we could not create 2 overrides with the same tenant workflow.

);
}

@Put('/workflows/:workflowId/tenants/:tenantIdentifier')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am still not sure if we should use here tenantIdentifier or _tenantId.

Comment on lines +86 to +87
const message = workflowOverride ? 'Workflow is not active by workflow override' : 'Workflow is not active';
Logger.log(message, LOG_CONTEXT);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wonder if we should log more here because at the moment we do not and if we would want to debug something we would be missing the information as to why the trigger failed.

Copy link
Contributor

@LetItRock LetItRock left a comment

Choose a reason for hiding this comment

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

looks good! 🚀

@djabarovgeorge djabarovgeorge merged commit 7ff9f70 into next Nov 16, 2023
21 of 23 checks passed
@djabarovgeorge djabarovgeorge deleted the workflow-override branch November 16, 2023 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants