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

Don't require ticket status and type #162

Merged
merged 3 commits into from
Jul 2, 2024

Conversation

laurenleach
Copy link
Contributor

  • We don't require status but if the schema didn't include status we were still erroring
  • Change ticket type to not be required since not all ticket systems require type

@@ -265,10 +265,11 @@ func ValidateTicket(ctx context.Context, schema *v2.TicketSchema, ticket *v2.Tic
}

// Look for a matching ticket type
foundMatch = false
foundMatch = ticket.Type == nil
for _, tType := range schema.GetTypes() {
if ticket.Type == nil {
Copy link
Contributor

@mgaeta mgaeta Jun 26, 2024

Choose a reason for hiding this comment

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

The code block

if ticket.Type == nil {
	foundMatch = true
	break
}

can be removed from the loop:

// Look for a matching status
// Status is not required
foundMatch := ticket.Status == nil
if !foundMatch {
  for _, status := range schema.GetStatuses() {
    if ticket.Status.GetId() == status.GetId() {
	    foundMatch = true
	    break
    }
  }
}

@@ -265,10 +265,11 @@ func ValidateTicket(ctx context.Context, schema *v2.TicketSchema, ticket *v2.Tic
}

// Look for a matching ticket type
foundMatch = false
foundMatch = ticket.Type == nil
Copy link

Choose a reason for hiding this comment

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

I might add a more specific comment here and above to explain why we do this, if a ticket doesn't have a status or a type we don't need to validate, skip the loop in this case

@@ -265,10 +265,11 @@ func ValidateTicket(ctx context.Context, schema *v2.TicketSchema, ticket *v2.Tic
}

// Look for a matching ticket type
foundMatch = false
Copy link

Choose a reason for hiding this comment

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

hasValidTicketType

@@ -245,7 +245,7 @@ func ValidateTicket(ctx context.Context, schema *v2.TicketSchema, ticket *v2.Tic
l := ctxzap.Extract(ctx)

// Look for a matching status
foundMatch := false
Copy link

Choose a reason for hiding this comment

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

validTicketStatus

@laurenleach laurenleach force-pushed the lauren/tickets-dont-require-status-and-type branch from b277b15 to 706279c Compare July 2, 2024 18:40
@laurenleach laurenleach merged commit fd4aee6 into main Jul 2, 2024
4 checks passed
@laurenleach laurenleach deleted the lauren/tickets-dont-require-status-and-type branch July 2, 2024 18:44
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.

3 participants