#1302158: PDO exception when saving where more than one path section used in alias pattern is fixed now, so this can go away.

CommentFileSizeAuthor
#7 pathauto-1988226-7.patch2 KBpancho
#5 pathauto-1988226-5.patch2.11 KBpancho
#3 pathauto-1988226-1.patch1.73 KBpancho

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Postponed (maintainer needs more info)

I'm not sure what you mean. That issue was fixed *by adding* the try/catch. The real issue that needs to be solved is #1003788: PostgreSQL: PDOException:Invalid text representation when attempting to load an entity with a string or non-scalar ID which is still pending for core.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Postponed (maintainer needs more info) » Fixed

I see now the actual link in the @todo was confusing. It should have linked to #1003788: PostgreSQL: PDOException:Invalid text representation when attempting to load an entity with a string or non-scalar ID, not the internal issue in Pathauto.
http://drupalcode.org/project/pathauto.git/commit/655e451

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented
Status: Needs work » Needs review
StatusFileSize
new pathauto-1988226-1.patch1.73 KB

[edit:]
Oops, I'm sorry - didn't reload the page.
I also should have read that issue.
Please ignore this patch.

Status: Fixed » Needs work

The last submitted patch, pathauto-1988226-1.patch, failed testing.

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented
Status: Needs work » Needs review
StatusFileSize
new pathauto-1988226-5.patch2.11 KB

So leaving the try... catch in.
But in case an exception has been caught, we don't want to return FALSE but at least continue checking the filesystem. Fixed that.

I also did some more rewording in the doc, because it didn't reflect the fact that we're checking both the menu system and the filesystem.
Also, the wording "Verify if the given path is a valid menu callback" is misleading. It's not like we want to make sure that it already exists, rather we want to make sure it doesn't.

IMHO, the function should also be renamed to something like _pathauto_path_exists() or _pathauto_check_path(), but I did not yet roll that in.

Hopefully, the testbot is able to apply this one correctly - don't know what happened to the other patch.

Title: Remove try ... catch in _pathauto_path_is_callback() » Refactor _pathauto_path_is_callback()
Status: Needs review » Needs work

The last submitted patch, pathauto-1988226-5.patch, failed testing.

pancho’s picture

pancho
Location UTC+2 🇪🇺 EU
commented
Category: task » bug
StatusFileSize
new pathauto-1988226-7.patch2 KB

Ah okay, now it should be fine. For explanation, see #5.
Escaping the filesystem check makes this a bug.

  • Dave Reid committed 655e451 on 8.x-1.x 
    Issue #1988226: Fixed @todo link in _pathauto_path_is_callback to...
  • Dave Reid committed 6644c8c on 8.x-1.x
    Revert "Issue #1988226: Fixed @todo link in _pathauto_path_is_callback...
  • Dave Reid committed d758135 on 8.x-1.x
    Issue #1988226: Fixed @todo link in _pathauto_path_is_callback to...

  • Dave Reid committed 655e451 on 8.x-1.x 
    Issue #1988226: Fixed @todo link in _pathauto_path_is_callback to...
  • Dave Reid committed 6644c8c on 8.x-1.x
    Revert "Issue #1988226: Fixed @todo link in _pathauto_path_is_callback...
  • Dave Reid committed d758135 on 8.x-1.x
    Issue #1988226: Fixed @todo link in _pathauto_path_is_callback to...
mably’s picture

mably commented
Issue summary: View changes
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

mably’s picture

mably commented
Status: Fixed » Closed (fixed)