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

EZP-30911: Added option to pass location-id parameter to URL alias regeneration Command #2776

Merged
merged 6 commits into from
Sep 26, 2019

Conversation

mateuszbieniek
Copy link
Contributor

@mateuszbieniek mateuszbieniek commented Sep 25, 2019

Question Answer
JIRA issue EZP-30911
Improvement yes
New feature no
Target version 7.5
BC breaks no
Tests pass yes
Doc needed yes

This PR adds an option to pass array option location-id so regeneration of URL aliases can be done for only specific locations, like this:
php bin/console ezplatform:urls:regenerate-aliases --location-id=1 --location-id=2 --location-id=100

TODO:

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

@mateuszbieniek mateuszbieniek changed the title EZP-30911: Command to regenerate URL aliases should allow fixing aliases only for specified Locations Added option to pass location-id parameter to URL alias regeneration Command Sep 25, 2019
@mateuszbieniek mateuszbieniek changed the title Added option to pass location-id parameter to URL alias regeneration Command EZP-30911: Added option to pass location-id parameter to URL alias regeneration Command Sep 25, 2019
Copy link
Contributor

@andrerom andrerom left a comment

Choose a reason for hiding this comment

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

But this is probably also relevant for 1.13 right?

@mateuszbieniek
Copy link
Contributor Author

mateuszbieniek commented Sep 25, 2019

@andrerom not sure, as v1 and v2 commands for regenerations are quite different (or am I misremembering something?)

@alongosz
Copy link
Member

But this is probably also relevant for 1.13 right?

It's kind of a feature, so I'd recommend upgrading to 2.5 to be able to use it for specific Locations. Moreover v1 does not support strict types, so refactoring would be quite extensive for method parameters and return types.

@andrerom
Copy link
Contributor

ok, if needed we can always cherry pick and backport 👍

Copy link
Member

@mnocon mnocon left a comment

Choose a reason for hiding this comment

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

Assuming the following Content Items structure:

  • root
    • parent1 (location id: 57)
      -- child1 (location id: 58)

And following ezurlalias_ml entries:

+--------+----------------------------------+-----------+-------------+-----------------+----+----------+-------------+-----------+------+----------------------+
| parent | text_md5                         | action    | action_type | alias_redirects | id | is_alias | is_original | lang_mask | link | text                 |
+--------+----------------------------------+-----------+-------------+-----------------+----+----------+-------------+-----------+------+----------------------+
|      0 | 34f83b4b453db075f374fa73365b8283 | eznode:57 | eznode      |               0 | 41 |        0 |           1 |         3 |   41 | parent1              |
|     41 | 27cfbe367a1294dc60aef746fb69e797 | eznode:58 | eznode      |               0 | 42 |        0 |           1 |         3 |   42 | child1               |

When I corrupt my database with:
delete from ezurlalias_ml where id=41;
(delete url alias for parent1)

and run command:
php bin/console ezplatform:urls:regenerate-aliases --location-id=57
(regenerate urlalias for parent1)

the result is:

+--------+----------------------------------+-----------+-------------+-----------------+----+----------+-------------+-----------+------+----------------------+
| parent | text_md5                         | action    | action_type | alias_redirects | id | is_alias | is_original | lang_mask | link | text                 |
+--------+----------------------------------+-----------+-------------+-----------------+----+----------+-------------+-----------+------+----------------------+
|      0 | 34f83b4b453db075f374fa73365b8283 | eznode:57 | eznode      |               0 | 54 |        0 |           1 |         3 |   54 | parent1              |

There are no entries for subitems, meaning that:

  • existing urlalias for child1 has been deleted (shouldn't it be recreated with the new parent value of 54?)

I think it's extremely easy to corrupt the database with the location-id parameter - if you use it on a location which has children then their urlaliases will break, so you have to use the command for all children as well.

Should it take the whole subtree into account or should it be even possible to run this command for location with children?

Copy link
Member

@mnocon mnocon left a comment

Choose a reason for hiding this comment

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

We've discussed this in private and decided that it can stay that way, when regenerating locations that have children use have to inspect the results and regenerate the rest of the subtree (manually) if needed

@alongosz alongosz merged commit d5fa5fa into ezsystems:7.5 Sep 26, 2019
@alongosz
Copy link
Member

@mateuszbieniek could you merge up?

@mateuszbieniek
Copy link
Contributor Author

@alongosz done in f81af13

@mateuszbieniek mateuszbieniek deleted the EZP-30911 branch September 27, 2019 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

7 participants