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

2.x Rename the entity base table #18

Open
wants to merge 14 commits into
base: 8.x-2.x
Choose a base branch
from

Conversation

WengerK
Copy link
Member

@WengerK WengerK commented May 1, 2020

💬 Describe the pull request

Rename base table from backerymails_sended_mail to backerymails_sent_mails

  • Update existing views
  • Update the entity
  • Migrate data from old to new table
  • Drop table when everything has been migated
  • Upgrade path coverage

🗃️ Issues

This pull request is related to :

🔢 To Review

Steps to review the PR:

  1. drush updatedb -y

📸 Pay attention to

  • All existing data in the previous table should be migrated
  • People who install the module for the 1st time should not need the updatedb, therefor nothing should break when the previous table does not exists
  • The Backerymail views should be updated with the new base-table name

@WengerK WengerK linked an issue May 1, 2020 that may be closed by this pull request
backerymails.post_update.php Outdated Show resolved Hide resolved
backerymails.post_update.php Show resolved Hide resolved
backerymails.post_update.php Outdated Show resolved Hide resolved
if ($database->schema()->tableExists('backerymails_sended_mail') && $database->schema()->tableExists('backerymails_sent_mails')) {
$query = $database->select('backerymails_sended_mail', 'sent_mails')
->fields('sent_mails');
$tuples = $query->execute()->fetchAll();
Copy link
Member

Choose a reason for hiding this comment

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

Hard to tell if we need to Batch API here (and the use of $sandbox argument). Did you test it with production data?

Copy link
Member Author

Choose a reason for hiding this comment

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

I try but on small project, your're right I will try on a huge database


$this->assertEquals(1, $database->query('SELECT count(*) FROM {backerymails_sent_mails}')->fetchField());
$backerymails = BackerymailsEntity::loadMultiple();
$this->assertCount(1, $backerymails);
Copy link
Member

Choose a reason for hiding this comment

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

How do we now that the hook post update is skipped ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed the tests, the post update is not skipped it will always, run if the old table still exists (but we remove that table at the end of psot_udate).

@WengerK WengerK changed the base branch from 8.x-1.x to 8.x-2.x June 28, 2020 15:49
function backerymails_post_update_8001_migrate_data(&$sandbox = NULL) {
$database = Database::getConnection();

if ($database->schema()->tableExists('backerymails_sended_mail') && $database->schema()->tableExists('backerymails_sent_mails')) {
Copy link
Member Author

Choose a reason for hiding this comment

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

use early return that explain:

as post_update will always run independent of hook_update, but only once. Ensure we don't run migrate if the module was already installed postschema 8001 (we may tests that instead of tableexists)

@WengerK WengerK mentioned this pull request Apr 10, 2024
@WengerK WengerK changed the base branch from 8.x-2.x to 3.0.x July 5, 2024 14:54
@WengerK WengerK changed the base branch from 3.0.x to 8.x-2.x July 5, 2024 15:04
@WengerK WengerK changed the title Rename the entity base table 2.x Rename the entity base table Jul 5, 2024
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.

[Drupal - Backerymails] - Refactoring & Migrations Table name incorrect
2 participants