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.

Sign up for GitHub

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

Jump to bottom

IBX-4906: Added MigrateRichTextNamespacesCommand #77

Merged
merged 35 commits into from
Mar 23, 2023
Merged

IBX-4906: Added MigrateRichTextNamespacesCommand #77

merged 35 commits into from
Mar 23, 2023

Conversation

ciastektk
Copy link
Contributor

@ciastektk ciastektk commented Feb 28, 2023 •
edited
Loading

Question Answer
JIRA issue IBX-4906
Required by ibexa/page-builder#206
Type improvement
Target Ibexa version v4.4+
BC breaks no

This PR provides command to migrate RichText namespaces based on implemented Gateways.

Namespaces:

Namespaces v3 Namespaces v4
xmlns:ezxhtml="http://ez.no/xmlns/ezpublish/docbook/xhtml" xmlns:ezxhtml="http://ibexa.co/xmlns/dxp/docbook/xhtml"
xmlns:ezcustom="http://ez.no/xmlns/ezpublish/docbook/custom" xmlns:ezcustom="http://ibexa.co/xmlns/dxp/docbook/custom"
ezxhtml:class="ez-embed-type-image" ezxhtml:class="ibexa-embed-type-image"
xmlns:ez="http://ez.no/xmlns/ezpublish/docbook" xmlns:ez="http://ibexa.co/xmlns/dxp/docbook"
xmlns:a="http://ez.no/xmlns/annotation" xmlns:a="http://ibexa.co/xmlns/annotation"
xmlns:m="http://ez.no/xmlns/module" xmlns:m="http://ibexa.co/xmlns/module"

TODO:

  • Fix a bug.
  • Implement tests.
  • Fix new code according to Coding Standards ($ composer fix-cs).
  • Ask for Code Review.

@vidarl vidarl mentioned this pull request Mar 3, 2023
Closed
4 tasks
@glye
Copy link

glye commented Mar 3, 2023

Community thread about the missing migration, with a request for doc. Good feedback, it looks like.
https://ezcommunity.slack.com/archives/C032PD5N9UY/p1677769664501289?thread_ts=1677768685.625069&cid=C032PD5N9UY

ciastektk and others added 23 commits March 9, 2023 09:01
@ciastektk
Added persistence layer - Gateway and Handler
17ada58
@ciastektk
Added namespaces migration map parameter
d986f0f
@ciastektk
Added MigrateRichTextNamespacesCommand
2cb1c48
@ciastektk
Added ext-pdo to composer.json
9056c83
@ciastektk
Added error related to Doctrine QueryBuilder::execute to ignored erro…
674cb3d 
…rs in phpstan.neon
@ciastektk
Added GatewayInterface
af4d800
@ciastektk
Added missing dependencies
c1d0b73
@ciastektk
Added integration tests setup
7b55645
@ciastektk
Added MigrateRichTextNamespaces/Gateway/DoctrineDatabaseTest
2aa6673
@ciastektk
Declared GatewayInterface as a service
187063b
@ciastektk
Fixed CS
8cc163c
@alongosz @ciastektk
fixup! Added missing dependencies
dafe360
@alongosz @ciastektk
[Composer] Dropped redundant friendsofphp/php-cs-fixer requirement
8458462
@alongosz @ciastektk
Included IbexaTestKernel-based tests to composer integration script
be39a3c
@alongosz @ciastektk
[Tests] Added integration test coverage
8385b05
@ciastektk
Fixed update query - added nested REPLACE for update value
aacc90f
@ciastektk
Fixed DoctrineDatabaseTest
e9a6d3d
@ciastektk
Dropped ExceptionConversion
17fb3cb
@ciastektk
Moved AbstractGateway and GatewayInterface to constracts
598fc23
@ciastektk
Adjusted Handler to iterate over all implemented Gateways
07a184e
@ciastektk
Fixed tests
2663e52
@ciastektk
Fixed boostrap.php for integration tests
4897e22
@ciastektk
Fixed DoctrineDatabaseTest
ddf506a
@ciastektk ciastektk force-pushed the ibx-4906-added-migrate-rich-text-namespaces-command branch from 42522f5 to ddf506a Compare March 9, 2023 08:01
@ciastektk ciastektk requested review from konradoboza and a team March 10, 2023 08:12
@ciastektk ciastektk force-pushed the ibx-4906-added-migrate-rich-text-namespaces-command branch from a7dd129 to c689061 Compare March 10, 2023 08:39
@ciastektk ciastektk requested a review from kisztof March 10, 2023 08:42
ciastektk and others added 2 commits March 10, 2023 09:45
alongosz
alongosz requested changes Mar 10, 2023
View reviewed changes
composer.json Outdated Show resolved Hide resolved
tests/integration/bootstrap.php Outdated Show resolved Hide resolved
src/bundle/Resources/config/default_settings.yaml

# RichText namespaces map to migrate
ibexa.field_type.rich_text.namespaces_migration_map:
'xmlns:ezxhtml="http://ez.no/xmlns/ezpublish/docbook/xhtml"': 'xmlns:ezxhtml="http://ibexa.co/xmlns/dxp/docbook/xhtml"'
Copy link
Member

Choose a reason for hiding this comment

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

Should ezxhtml be still in use?

Yes if technically possible. I'm not sure how this would affect OE

Okay, so it indeed is a backend issue not an OE one. The same way we updated xmlns URI we should update XML namespace names:

  • xmlns:ez
  • xmlns:ezxhtml
  • xmlns:ezcustom

to be eg. ibx and provide upgrade script like this one (ideally just extending it). It would be good to include it right now to reduce possible future complexity, but it's not as simple as just upgrading field values in the database. The entire ezpublish.rng needs to be refactored, including also other legacy naming occurrences (not to mention file name itself, which is a BC issue).

FYI @webhdx @adamwojs maybe we need to schedule additional follow-up work for this.

src/lib/Persistence/Legacy/MigrateRichTextNamespaces/Gateway/DoctrineDatabase.php Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Mar 10, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 5 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@ciastektk ciastektk requested a review from alongosz March 10, 2023 14:11
@bogusez bogusez self-assigned this Mar 14, 2023
This was referenced Mar 14, 2023
Closed
Closed
@bogusez
Copy link

bogusez commented Mar 23, 2023

Regression tests passed:
ibexa/experience#155
ibexa/commerce#233

@alongosz alongosz merged commit bf45e57 into 4.4 Mar 23, 2023
@alongosz alongosz deleted the ibx-4906-added-migrate-rich-text-namespaces-command branch March 23, 2023 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dew326 dew326 dew326 left review comments

@vidarl vidarl vidarl approved these changes

@alongosz alongosz alongosz approved these changes

@konradoboza konradoboza konradoboza approved these changes

@bogusez bogusez bogusez approved these changes

@kisztof kisztof Awaiting requested review from kisztof

Assignees

@adamwojs adamwojs

@alongosz alongosz

@bogusez bogusez

Labels
Improvement QA approved Ready for MERGE
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
9 participants
@ciastektk @glye @bogusez @vidarl @kisztof @alongosz @dew326 @konradoboza @adamwojs