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

Fail validation for blank changeset ids (DAT-18773) #6472

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

abrackx
Copy link
Contributor

@abrackx abrackx commented Oct 30, 2024

Impact

  • Bug fix (non-breaking change which fixes expected existing functionality)
  • Enhancement/New feature (adds functionality without impacting existing logic)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

If a changeset id is entirely blank (one or more spaces) fail validation.

Things to be aware of

Things to worry about

Additional Context

@@ -184,7 +184,7 @@ protected void validateChange(ChangeSet changeSet, Database database, Change cha

private boolean areChangeSetAttributesValid(ChangeSet changeSet) {
boolean authorEmpty = StringUtils.isEmpty(changeSet.getAuthor());
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we introduce another validation for the author as well, since it also uses isempty? Maybe in another ticket?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree. If we apply the same logic for author, I think we can remove the strict validation we added some time ago.

StevenMassaro
StevenMassaro previously approved these changes Oct 30, 2024
@@ -184,7 +184,7 @@ protected void validateChange(ChangeSet changeSet, Database database, Change cha

private boolean areChangeSetAttributesValid(ChangeSet changeSet) {
boolean authorEmpty = StringUtils.isEmpty(changeSet.getAuthor());
boolean idEmpty = StringUtils.isEmpty(changeSet.getId());
boolean idEmpty = StringUtils.isBlank(changeSet.getId());
Copy link
Collaborator

Choose a reason for hiding this comment

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

@abrackx but this is validation all changesets, including existing ones right? IT's not in the ticket description, but ther eis a comment stating:
We will not fix changesets that have been already deployed with empty ChangeSetIDs
So if the changeset was already applied we should not validate it, otherwise the user will have to fix it and will have a changeset mismatch.
can we do it here?

@StevenMassaro StevenMassaro dismissed their stale review October 30, 2024 19:10

filipe's comments need to be addressed

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.

4 participants