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

BUGFIX: add dimensionshash migration to postgres #4429

Conversation

Jonathan2022Bausch
Copy link
Contributor

@Jonathan2022Bausch Jonathan2022Bausch commented Aug 1, 2023

Regression #3279

We need to generate add dimensions hash to node event model in order to be able to save new nodes as we do in MySQL: Neos.Neos/Migrations/Mysql/Version20210125134503.php

Upgrade instructions

Run ./flow doctrine:migrate

Review instructions

You want to make sure that the dimensions table has valid values. If not you might want to add a custom migration like this one:

<?php
declare(strict_types=1);

namespace Neos\Flow\Persistence\Doctrine\Migrations;

use Doctrine\Migrations\AbstractMigration;
use Doctrine\DBAL\Schema\Schema;

class Version20230727164559 extends AbstractMigration
{
    public function getDescription(): string
    {
        return 'Correct null entries in event log caused by automated import';
    }

    public function up(Schema $schema): void
    {
        $this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'postgresql', 'Migration can only be executed safely on \'postgresql\'.');

        $this->addSql('UPDATE neos_neos_eventlog_domain_model_event SET dimension = NULL WHERE dimension = :dimension AND dtype = :dtype;', ['dimension' => 'N;', 'dtype' => 'ttree_contentrepositoryimporter_event']);
    }

    public function down(Schema $schema): void
    {
        $this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'postgresql', 'Migration can only be executed safely on \'postgresql\'.');

        $this->addSql('UPDATE neos_neos_eventlog_domain_model_event SET dimension = :dimension WHERE dimension IS NULL AND dtype = :dtype;', ['dimension' => 'N;', 'dtype' => 'ttree_contentrepositoryimporter_event']);
    }
}

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

@github-actions github-actions bot added the 8.3 label Aug 1, 2023
@Jonathan2022Bausch Jonathan2022Bausch changed the title 8.3 add dimensionshash migration to postgres BUGFIX 8.3 add dimensionshash migration to postgres Aug 1, 2023
@crydotsnake
Copy link
Member

Hello,

Thanks for your contribution!

Is this bugfix related too 8.3? otherwise this should go in the 7.3 branch.

@Jonathan2022Bausch
Copy link
Contributor Author

Hello,

Thanks for your contribution!

Is this bugfix related too 8.3? otherwise this should go in the 7.3 branch.

I am currently developing with 8.3 but I am open to merge this into 7.3

@Jonathan2022Bausch Jonathan2022Bausch changed the base branch from 8.3 to 7.3 August 2, 2023 07:54
@github-actions github-actions bot added 7.3 and removed 8.3 labels Aug 2, 2023
@crydotsnake crydotsnake changed the title BUGFIX 8.3 add dimensionshash migration to postgres BUGFIX: add dimensionshash migration to postgres Aug 5, 2023
@crydotsnake crydotsnake added the Bug label Aug 5, 2023
@crydotsnake
Copy link
Member

Hm. I believe something went wrong during the rebase. Can you fix this? Otherwise, you can create a new branch based on 7.3 and open a new pull request :)

@markusguenther markusguenther force-pushed the 8.3-dimensionhash-migration-to-postgres branch from 6b27c57 to bd60a11 Compare August 17, 2023 13:13
@markusguenther
Copy link
Member

Adjusted the PR and now it is based on Neos 7.3.
After your PR I also understand your comment in PR (#3986) better.
I did not enable the history locally.

@markusguenther
Copy link
Member

I was wrong and @andrehoffmann30 added the hash in Neos 8.3, and we just forgot the Postgres Migration. So will adjust that to 8.3 again.

@dlubitz
Copy link
Contributor

dlubitz commented Jan 5, 2024

Looks good to me! But we need to change it back to 8.3, as this was introduced for MySQL in 8.3 too.

@markusguenther markusguenther force-pushed the 8.3-dimensionhash-migration-to-postgres branch from bd60a11 to 8df08fa Compare January 5, 2024 12:23
@markusguenther markusguenther changed the base branch from 7.3 to 8.3 January 5, 2024 12:24
@github-actions github-actions bot added 8.3 and removed 7.3 labels Jan 5, 2024
@markusguenther
Copy link
Member

We discussed that PR in our CR Weekly and agreed on merging that change. In Neos 9.0 the EventLog has been removed.

@markusguenther markusguenther merged commit 191f96c into neos:8.3 Jan 5, 2024
3 checks passed
@markusguenther
Copy link
Member

Thanks for Your contribution @Jonathan2022Bausch 🥇💙

@markusguenther
Copy link
Member

Your first contribution has been released now 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants