User is able to create pages with same URL alias

I can confirm that's an issue.

In ApiAutoSaveController::post we're validating the entities to be saved against the existing entities, but not against each other.

IMHO, that should trigger an exception and rollback the saves

    // Either everything must be published, or nothing at all.
    try {
      $transaction = $this->database->startTransaction();
      foreach ($entities as $entity) {
        $entity->save();
        $this->autoSaveManager->delete($entity);
      }
    }
    catch (\Exception $e) {
      if (isset($transaction)) {
        $transaction->rollBack();
      }
      Error::logException($this->logger, $e);
      throw $e;
    }

I think that's not a data loss, but could it be close to?

Let's start with test coverage 🙏

PathAlias includes

  constraints: [
    'UniquePathAlias' => [],
  ],

but we don't enforce this at the db level, so it will silently happen if we publish both entity aliases on the same transaction (which matches some already reported issues on pathauto and stackoverflow).

If we wanted to fix this in core at the db level, we would include in PathAliasStorageSchema

    $schema[$base_table]['unique keys'] += [
      'path_alias__alias_langcode_status' => ['alias', 'langcode', 'status'],
    ];

If we want to workaround this, we can override the UniquePathAliasConstraintValidator constraint validator, and ensure it checks auto-saved data. But I'm pretty sure we want a consistent solution and not having to override every other storage/constraint validator to be aware of AutoSaveManager.

Not sure what we want as a path (pun intended) forward.

Test coverage done and failing (expected) for 2 nodes with the same path alias.

https://git.drupalcode.org/project/experience_builder/-/jobs/5920291#L585

tedbow → changed the visibility of the branch 3536247-detect-collisions-autosave to hidden.

Left a review 👍️

I've updated the title to surface the real issue.

Revert issue title thanks to that PathAlias is only validating on form submit.

We decided that we're going to validate only the entities ($entity->validate) right before the $entity->save() to still validate Constraints and don't execute the form validate.

I'll work on that and adapt the tests.

@isholgueras assigning back to you I think there is some merge problem with the MR it now has 63 files changed

I don't why gitlab was showing me so many file changes an hour or so ago🤔

Failing test-only CI 👍

Left a couple of comments on the MR

Per #22.

I think Nacho having addressed Lee’s review means @tedbow can RTBC & merge 😊🤞

Code looks good. I have to update summary because we are not solving the initial problem reported and no using the solution in the summary

but I will then RTBC and merge

Thanks all!

Automatically closed - issue fixed for 2 weeks with no activity.