-
Notifications
You must be signed in to change notification settings - Fork 130
Resolved #4399 where Structure URI for cloned entry was built differently from cloned URL Title #4819
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
Conversation
…ntly from cloned URL Title
robinsowell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah - the uri for structure was ending up without the copy on it- so basically a duplicate (which I thought wouldn't validate, but did and that might be another issue).
Basically- if I dump $_POST['structure__uri'] = $values['uri'] = $value; and die right before it returns- I get:
"copy-/a-photograph-for-the-ages"
because we're putting that leading slash on the value before we add 'copy' to it. Structure doesn't allow slashes inside the uri so I guess the whole copy bit gets dropped on save and you end up with just the original 'a-photograph-for-the-ages'.
|
@robinsowell any chance you can test this again? |
|
Hrm. So definitely solved the first issue from testing and everything seem ok except for 1 minor inconsistency which ... I don't think is a problem, but is an inconsistency. I create an entry, structure uri is blog-child-dos-uri and it is set as the child of another page. I clone that entry, cloned entry has structure uri of blog-child-dos-uri-1 and it has the proper parent selected. See how it's blog-child-dos-uri-1? If I do the same thing but it's NOT a child, original uri is blog-top-uno-uri and copy uri is copy-blog-top-uno-uri. In other words- always the clone url_title and structure/page uri is created with copy-whatever. In the case of a subpage, it's not doing that. Again- not sure whether this is a problem or not. @TomJaeger your thoughts? |
robinsowell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not requesting change per se, but I'd like someone else to weigh in on inconsistency noted in my last comment.
|
I would say we need to change this so it matches the URL title regardless of where it is in the structure tree |
|
The fix for this is now in place |
robinsowell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed for me!
I did discover this bug while testing: #4959 BUT that happens without these changes as well, so different issue.
|
Added a code review request from either @matthewjohns0n and @bryannielsen Im wondering if we need to change some of the logic in the sql.structure file code updates. |
Resolved #4399 where Structure URI for cloned entry was built differently from cloned URL Title
Note: this is not really a bug as such, as we still have unique value and editor would still probably want to change it... But it's nice when both are built using same pattern