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

use maxlength instead #2562

Closed
wants to merge 2 commits into from
Closed

use maxlength instead #2562

wants to merge 2 commits into from

Conversation

GraceGardner
Copy link
Contributor

Description & motivation 💭

Add maxlength to workflowid. I went with 36 as the max length because that's what I guess is UUID standard. Original ticket was suggesting a max for an INT, but it looks like since then we changed from an int to a UUID.

Testing 🧪

Go to start a work flow, try to put in an id longer than 36 characters. It now maxes out at 36 and does not allow the user to put in a larger input.

How was this tested 👻

  • Manual testing
  • E2E tests added
  • Unit tests added

Steps for others to test: 🚶🏽‍♂️🚶🏽‍♀️

Go to start a work flow, try to put in an id longer than 36 characters. It now maxes out at 36 and does not allow the user to put in a larger input.

Checklists

Draft Checklist

Merge Checklist

Issue(s) closed

DT-2701

Docs

Any docs updates needed?

@GraceGardner GraceGardner requested a review from a team as a code owner February 19, 2025 08:13
@GraceGardner GraceGardner requested review from GiantRobots and removed request for a team February 19, 2025 08:13
Copy link

vercel bot commented Feb 19, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
holocene ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 19, 2025 4:48pm

@Alex-Tideman
Copy link
Contributor

Let's check out the ticket again, it's talking about a Search Attribute with Int type not the Workflow Type field which is a string.

@Alex-Tideman Alex-Tideman self-requested a review February 19, 2025 14:54
Copy link
Contributor

@Alex-Tideman Alex-Tideman left a comment

Choose a reason for hiding this comment

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

I'm a little surprised the type check didn't catch this. The prop is maxLength, not maxlength.

@@ -216,6 +216,7 @@
bind:value={workflowId}
label="Workflow ID"
class="w-full grow"
maxLength={36}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just noting that whatever we do here we'll likely also need to do for the Workflow ID input in the schedule form.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, looking at the original issue here and I think it lies with the custom search attribute input not with the inputs on these forms.

Copy link
Contributor

Choose a reason for hiding this comment

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

^ Yup this is for the Int type input for a Search Attribute

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you both!

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