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

Bulk generate doesn't create paths #121

Closed
tormu opened this issue Jan 5, 2016 · 14 comments
Closed

Bulk generate doesn't create paths #121

tormu opened this issue Jan 5, 2016 · 14 comments

Comments

@tormu
Copy link

tormu commented Jan 5, 2016

As instructed at https://www.drupal.org/node/2573683#comment-10710038, I updated token to latest dev, installed newest dev of ctools, updated the pathauto to latest code from https://github.com/md-systems/pathauto/tree/8.x-1.x and ran drush updb

Then I deleted all my paths at admin/config/search/path/delete_bulk
Then went to admin/config/search/path/update_bulk to rebuild them all

Result was that the ajax thingy went ok, giving steady flow of messages like "Updated alias for Content 80" and so on..
but in the end I get message "No new URL aliases to generate."

No errors in watchdog either.
Paths are created ok if I go to edit content and save them. However, I have to manually check the "Generate automatic URL alias" in the node, which sounds a bit weird. This checkbox is ticked if I create a node from scratch, but for content that I mass-deleted the paths it's not. Maybe this is the reason the bulk doesn't work?

If I go to the /admin/content and make a bulk operation to update aliases, that works.

@Berdir
Copy link
Member

Berdir commented Jan 5, 2016

See #62, this is pretty much a duplicate of that.

For certain aliases/path combinations, we get into troubles because it's always matching existing routes. I don't know yet how to fix that.

@Berdir Berdir closed this as completed Jan 5, 2016
@stewest
Copy link

stewest commented Jan 6, 2016

Hi there. I think this may be slightly different to my issues at #62 and rather extends it.

All my nodes did not have "Generate automatic URL alias" checkbox checked. So once I sorted my Alias patterns out, during the bulk operations, I would also get "No new URL aliases to generate."

So, rather than going to each node (100s) to switch on the auto alias generation, I went to /admin/content, selected all and I used the bulk operations over there to "Update URL-Alias". That did generate the PathAuto url-aliases.

@flocondetoile
Copy link

Hi,

I met same issue. When I delete and rebuild all path alias, nodes lost their path alias, because the option "Generate automatic URL alias" was not anymore checked.

I made a restauration of the database, and checked that this option was well active for the nodes, before the delete/rebuild action.

Only 4 nodes (over ~100) retained their path alias. Trying to find the reason.

@Berdir
Copy link
Member

Berdir commented Jan 6, 2016

Could be related to the new pathauto_state feature that I merged in, maybe there's a bug there, or did you update from an existing site? When you edit those nodes, is the checkbox checked or not?

@flocondetoile
Copy link

Yes, if i edit thoses nodes (before delete/rebuild actions), the checkbox is checked.
After delete/rebuild alias, checkbox is not anymore checked.

Some nodes retained this option checked, and their path alias was fine.

Tests done on an existing site (migrating from D7 to D8). I will try on a fresh D8 install to reproduce it.

@flocondetoile
Copy link

Tests done with the dev version from this commit b749e06cafa6f41561e9a8a941d4e89f3e884b9a1

@flocondetoile
Copy link

Well,

On an existing site migrated from D7.
All nodes have the option "Generate automatic URL alias" checked.

The path alias option is unchecked and the alias deleted when i launch the bulk delete action only for nodes created before the installation of pathauto (or not edited after his installation). And so when I launch the rebuild bulk action, alias are not regenerated.

For nodes created after, or edited/saved whereas pathauto is enable, the bulk delete action erase the alias but the option "Generate automatic URL alias" is always checked and so alias are regenerated on the rebuild bulk action.

I have try to reproduce this on a fresh D8 without success by creating node before installation of pathauto, creating other node after. But then all alias are regenerated (option "Generate automatic URL alias" is always checked even after deleted all the alias). Node created before the pathauto installation have not the option "generate automatic alias" checked. Logic.

Seems to be specific to the site migrated. Nodes migrated seems to have the automatic generation option checked but seems to be not the case really.

During this tests, I notice an another issue, similar, and reproductible on both sites (old and the fresh install). If a node have a manual alias (option "Generate automatic URL alias" unchecked), the delete bulk operation erase this alias. And logically, alias are not regenerated.

Best

PS : tests done with the latest branch of pathauto (I clone the repositoty just before)

@tormu
Copy link
Author

tormu commented Jan 7, 2016

Our site is D8 from the beginning, but there's some burden from the past since the site was started during later betas. But the behaviour is similar - bulk delete seems to remove the "Generate automatic URL alias" checkbox even though it's checked before the delete operation.

@Berdir Berdir reopened this Jan 7, 2016
@Berdir
Copy link
Member

Berdir commented Jan 7, 2016

I think I incorrectly closed this one, confused it with another issue, sorry.

This is definitely related to the pathauto_state that we added. The point of that is to prevent the opposite. That bulk-generate does not force-generate an alias for nodes that have none or a custom alias assigned when being created.

It should still work if there is no explicit flag to not create an alias, but apparently something goes wrong there.

it would be helpful if someone could work on a test that fails. See Drupal\pathauto\PathautoState, I guess you could make this fail then when you extend PathautoBulkUpdateTest, to delete all aliases again, then also delete the records in state created by the PathautoState class (deleteAll() for our collection should do the trick) and then trigger the bulk-generate process again.

@flocondetoile
Copy link

I began to look to add additional tests in PathautoBulkUpdateTest. I think it's going to be complicated to make the test because the problem seems to be the mass delete operation. And so the test will always fail. The method submitForm of Class PathautoAdminDelete delete all aliases from the database with a simple db_delete() (as the utility method deleteAllAliases() do) wether or not the flag to not create alias is set. And then, if the flag is not set, the manual alias is deleted but not generated with the bulk operation (the normal behaviour). To test this, a simple way could be to change status message displayed after a bulk deletion by including the total number of aliases deleted by the bulk operation. What's your mind ?

@flocondetoile
Copy link

I will looking too for the test with the deletion of the state record. what is the normal behavior if the entity does not have a state record ? It seems that the flag "Generate automatic URL alias" is not checked.

@flocondetoile
Copy link

I found why nodes created before pathauto installation have the option "Generate automatic URL alias" checked but their aliases are not regenerated on bulk update operation. It's because the custom/manual alias set on these nodes matches the pattern defined. Otherwise this option is not checked.

In the PathautoState method getValue(), you set the value to CREATE if the node's alias matches the pattern for this content type

if (($path != $entity_path && $path == $pathauto_alias)) {
   $this->value = static::CREATE;
}

So we believe that the node have the option checked. But the state is not saved in the KeyValue table, and so when you mass delete aliases, these nodes don't have anymore custom alias and as the PathautoState have not been saved, the option "Generate automatic URL alias" is not anymore checked, because there is not anymore alias.

I don't know how manage this situation (for nodes created before pathauto installation, or in a more generic situation, for site migrated from D7) ? Should we persist the value as this ?

if (($path != $entity_path && $path == $pathauto_alias)) {
          $this->value = static::CREATE;
          // Entity alias is construct on the same pattern
          // And so we store the value
          // otherwise Entity seems to have pathauto enabled but this is not stored
         $this->persist();
}

Or is it a specific use case which have to be managed in a hook_install (or hook_updated_N) ?

But should Pathauto hypothesized that nodes with custom aliases which match pattern should have a PathautoState set to CREATE ?

Anyway, Pathauto should not mass delete aliases which are not managed by Pathauto (nodes don't have a PathautoState value). Il will add some test about this.

@Berdir
Copy link
Member

Berdir commented Jan 17, 2016

Yeah, that line is the root of the problem there. There are two things to think about:

I think if there is no state and no alias, then we should allow to create a new one (Which is different from having it explicitly set to FALSE). So that means the condition would be $path == $entity_path || $path == $pathauto_alias

I ported this feature from 7.x-1.x. The implementation is however completely different and hard to compare. Wondering if I introduced this in 8.x-1.x or if it's already in 7.x-1.x. There might be issues about it too there.

The second is the question if we should delete those that have no pathauto state or not. I think yes, Mass delete is IMHO something you very rarely use (you can just re-generate and it will respect that setting and update old aliases). Maybe clarify that in the documentation. Otherwise mass delete would become very slow as we'd have to load and check every entity.

@Berdir
Copy link
Member

Berdir commented Feb 17, 2016

This is handled in https://www.drupal.org/node/2660640

@Berdir Berdir closed this as completed Feb 17, 2016
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

No branches or pull requests

4 participants