With DBTNG we are extending the build in PHP PDO way of communication with different databases. The name for the build in method for rolling back transactions is PDO::rollBack() (with a capital B).
The corresponding method in D8 is called Drupal\Core\Database\Connection::rollback() (without the capital B). Lets correct that. This will not be an API change we can do this in a minor version.

The original find of this bug is from @axel.rutz, so please give him commit credits for it.

CommentFileSizeAuthor
#7 connection_rollback-2801899-7.patch12.92 KBshashikant_chauhan
#4 connection_rollback-2801899-4.patch581 bytesshashikant_chauhan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

daffie created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Version: 9.x-dev » 8.3.x-dev
Category: Bug report » Task

PHP is case insensitive so this wouldn't be an API change.

daffie’s picture

daffie CreditAttribution: daffie commented
Issue summary: View changes
Issue tags: +Novice

If somebody wants to take this on, then I will do the reviewing.

shashikant_chauhan’s picture

shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commented
FileSize
connection_rollback-2801899-4.patch581 bytes

adding patch

shashikant_chauhan’s picture

shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commented
Status: Active » Needs review
daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs review » Needs work

@shashikant_chauhan: Your patch looks good. The problem is all calls for that method also need to be renamed. Also in the documentation there are a lot of Database::rollback()'s instead of Database::rollBack(). Watch out that you do not change the word rollback. Like in: "Create a savepoint so we can rollback a failed query".

shashikant_chauhan’s picture

shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commented
Status: Needs work » Needs review
FileSize
connection_rollback-2801899-7.patch12.92 KB

updated the patch.

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs review » Reviewed & tested by the community

Looks good to me. All function calls for rollback are changed. No unnecessary changes. So, for me it is RTBC.

Thanks @shashikant_chauhan!

xjm credited axel.rutz.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm at Acquia commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

Looks good to me.

I think we should add a small CR though so modules know to update their incorrectly cased method calls or overrides. The CR can clarify that it will not break any existing code.

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs change record

Added a change record.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: connection_rollback-2801899-7.patch, failed testing.

daffie’s picture

daffie CreditAttribution: daffie commented
Status: Needs work » Reviewed & tested by the community

Back to RTBC.

  • catch committed 9f26c56 on 8.3.x 
    Issue #2801899 by shashikant_chauhan, daffie, axel.rutz: Rename Drupal\...
catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.3.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.