-
Notifications
You must be signed in to change notification settings - Fork 641
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
Add UID route token #3583
Add UID route token #3583
Conversation
@AugustMiller Have you tested both of these regexes with capital letters? @angrybrad if UUIDs are only including letters a-f, we should probably update StringHelper::isUUID() to match this regex. (Also not sure why the |
In its current state, it won't match capital letters… all the references I've seen to the pattern assume lower-case My intent was to match whatever is being output by Up to this point, I hadn't realized the “audit columns” were a Craft ActiveRecord extension, and not something that Yii provided! |
v4 UUID's allow for upper or lowercase, so the |
@AugustMiller can you check if the functionality still works as you'd expect with those changes? |
Unfortunately not—probably due to the subset of regular expressions that can actually be injected into a route? For example, the final route (taken from our app) ends up as this, in the Yii route debugger:
…which is skipped by: It seems like we might need an additional route-compatible pattern? Edit: Some quick searching revealed nothing about case-insensitive route expressions in Yii—I wonder if the regex itself just needs to be doubly-verbose? The other challenge is the beginning-of-line and end-of-line flags, which may also be incompatible. |
@AugustMiller yeah, you're correct. This should do it though (actually tested it works with a UID route this time): cbd3abe |
Works like a charm. 👌 |
Addresses #3582.
Sorry if this is too presumptuous—had the changes queued locally, so I figured it was worth committing!
✌️