Add path alias to user edit

Hey catch, I would like to tackle this. I am wondering, should the patch live in path or user module? Or a module of it's own?

And, merlinofchaos mentioned "toolbox" here: http://drupal.org/node/475968#comment-1641244

Is there already a helper module in place that deals solely with paths and url aliases?

I was looking at this one too as it's very similar to #476290: Add path alias to taxonomy term edit. It seems to me you should be able to achieve this just with path.module by using hook_user() and hook_form_alter() in a similar manner as is currently done with nodes. This is the approach I took to implement this for taxonomy.module (patch here: http://drupal.org/node/476290#comment-1853636).

Anyway, glad for the discussion and curious what the best approach will be.

yes catch, do you have a recommendation on where this extra code should reside? I am wondering if there is some type of policy or coding standards when dealing with core. Do we add code to path.module or to user.module - to handle this issue?

Here we go.

Next try.

cleaned form api (textfield doesn't support collaps[ed|ible]

tested and reviewed (code and functionality)

Are you sure that put these functions on path.module is the correct way?

Needs screenshot.

i hope this screenshot is that you're lookin' for :P

@Psicomante: Yes it needs to be in path.module because otherwise you'll have to check in user.module if the module "path" is installed. It would work but its cleaner to inject the path settings via form_alter in path.module.

I tested the patch from #9 and it looks good. I was able to add a url alias when editing a user (which worked), visit the url, remove the url, add it directly as a url alias in admin/settings/path and edit it again, and also remove it - that all worked fine :)

There were a few minor formatting issues with the comments - I've re-rolled with those fixed (thank you, coder module!).

trying this issue out as my first patch review

Hi catch!

I reviewed this patch (and this is my first review, btw)

The patch itself works. I was also able to apply the patch and then add an alias to user accounts. Visiting the alias also worked.

(I'm still figuring out how to make patches from my drupal install -- so I'm just pasting some code for you here.)

However, a few things are necessary for this patch, mostly comments.

1. Here is the beginning of a comments for the new function: _path_user_form_inject_path(&$form)

/**
* Add path settings to user register form.
*
* Returns fieldset with URL path settings.
* This allows a URL alias to specify an alternative URL by which the user's profile can be accessed.
*/
function _path_user_form_inject_path(&$form) {

2. This function could use more comments (what form elements you are adding and why) However, no one else in Drupal 7 has done this for any form altering functions. That seems kind of a bad practice, especially when there are dozens of form_alter calls and they all do different things. So, while I am not recommending that you add any more comments for this...I would personally like to find out why this is a common practice in Drupal.

path_form_user_profile_form_alter(&$form, &$form_alter) {

3. This function could use some documentation for the arguments (though this one is probably also a drupal-wide problem.) I ask because it is not immediately apparent what the &$edit variable is, or what the $category is....:

function path_user_submit(&$edit, &$account, $category = NULL) {

4. Is 'inject' the best word for a function name? (I'm not sure) There were not any other functions in d7 with that word in it. I did find the word 'inject' in the code, but it was in the doc-block for the function. Maybe you could change the function name from _path_user_form_inject_path _path_user_form_path ?

Here is a similar example:
/**
* Inject links into $node for attachments.
*/
function upload_node_links($node, $build_mode) {
$links = array();

Other things I thought about, but decided it was OK the way you did it (at least from my perspective)

a. setting access permissions to user 'create URL alias' - This makes sense because only users who can edit users can adjust the user alias. This is totally different than, say, adding taxonomy terms for users & classifying them.

b. not setting the default alias to 'user/useralias' -- i think it makes sense to create ANY alias you want - and that can be configured with pathauto

Other things to test
This module should be tested with pathauto, that pathauto filters will work with users.

unassigning this issue

@chachasikes
Thanks for your review.

@1.: Added comment to function.

@2.: Added some comment.
Why it (and what exactly) is "bad practice"? Using a helper function to add the form elements rather than inserting duplicate code?
Or do you mean the use of hook_form_FORM_ID_alter?

@3.: http://api.drupal.org/api/function/hook_user/7
hook_user_submit doesn't have its own document page, yet.

@4.: I've chosen "inject" because thats what it does. It injects some elements into an existing form. But I'm not nailed down to this wording ...

Patch works as advertised. Good idea!

+++ modules/path/path.module	9 Aug 2009 10:17:21 -0000
@@ -250,6 +250,110 @@ function path_form_alter(&$form, $form_s
+    // If we already have a path add its id so we can update the path rather than creating
+    // a new one after saving the profile.

Comments should wrap at 80 characters.

+++ modules/path/path.module	9 Aug 2009 10:17:21 -0000
@@ -250,6 +250,110 @@ function path_form_alter(&$form, $form_s
+  // Add custom submit handler to the user register form because there is no hook.

Comments should wrap at 80 characters.

+++ modules/path/path.module	9 Aug 2009 10:17:21 -0000
@@ -250,6 +250,110 @@ function path_form_alter(&$form, $form_s
+/**
+ * Custom submit handler for the user registration form to save an optional url alias.
+ */

Comments should wrap at 80 characters.

+++ modules/path/path.module	9 Aug 2009 10:17:21 -0000
@@ -250,6 +250,110 @@ function path_form_alter(&$form, $form_s
+  path_set_alias('user/' . $account->uid, isset($edit['path']) ? $edit['path'] : NULL, isset($edit['pid']) ? $edit['pid'] : NULL);

Can we expand this logic into multiple lines for increased code readability?

Other than that, looks pretty good.

New patch with updated comments.

Argh, this is because I can only use diff instead of cvs diff here. Will reroll later today.

Updated the patch (again).

Dear bot: please retest.

patch applied cleanly to head - reuploading for test bot

All cwgordons issues have been adressed, back to RTBC?

I don't agree with the approach taken here. Instead of introducing new functions that almost match the existing functions, the existing functions should be extended to work for more entities/object types.

Additionally, could I get clarification on the use case here?

Taxonomy terms I get. It's very conceivable that you have a free-tagging vocab where you don't really care what the URLs are, but in your "Product Type" vocabulary, you specifically want URLs like example.com/tshirts, example.com/videos, etc.

But users? Is there a use case for user/1, user/2, user/3, user/tom_the_frightening_toast?

Seems like this is a classic case where you'd want a pattern-based URL-generator like Pathauto, not a one-off thing, which would result in terribly inconsistent user profile URLs.

@webchick: A form widget to enter any URL alias for a user is a pre-condition for auto-generated aliases. You always need a way to override those.

Introducing a new tag for feature freeze: API clean-up.

Subscribing.

I think we'll likely accept a patch in Pathauto for D7 to add this to the user forms.

Given the new approach proposed in #29, is this still a Novice issue?

Suppose this should be done inline with #2201051: Convert path.module form alters to a field widget

There is patch which works with another patch from parent issue https://drupal.org/node/2201051#comment-8722973

Postponed to #2201051: Convert path.module form alters to a field widget

Continue work.

I tested patch, seems to work correctly.

Still no updates?

Reroll

I'm not sure sure this makes too much sense unless you use it in combination with pathauto and then possibly only for administrative users? You wouldn't want to give your users full control over the alias they can use? The only thing you might want to is have a field where they can for example control /user/[something], then you'd in turn use pathauto to generate the actual path.

What I have in mind is a feature for pathauto in 8.x that allows to simply check of which entity types should have this field, which at the same time would also enable them to have pathauto alias patterns.

The patch does not apply to latest drupal 8(branch 8.0.x) code base.

Re-rolled patch in #52.

The patch does not apply to latest drupal 8(branch 8.0.x) code base.

Reolled #55 patch.

er.pushpinderrana
Your patch is missing closing curly brace in 113 line
/core/modules/path/path.module

+++ b/core/modules/path/path.module
@@ -58,28 +58,25 @@ function path_permission() {
@@ -93,3 +90,23 @@ function path_entity_base_field_info(EntityTypeInterface $entity_type) {

@@ -93,3 +90,23 @@ function path_entity_base_field_info(EntityTypeInterface $entity_type) {
     return $fields;
   }
 }
+
+/**
+ * Returns form element for path settings.
+ */
+function path_settings_form_element(&$form) {
+  $form['path_settings'] = array(
+    '#type' => 'details',
+    '#title' => t('URL path settings'),
+    '#open' => !empty($form['path']['widget'][0]['alias']['#value']),
+    '#group' => 'advanced',
+    '#attributes' => array(
+      'class' => array('path-form'),
+    ),
+    '#attached' => array(
+      'library' => array('path/drupal.path'),
+    ),
+    '#weight' => 30,
+  );
+  $form['path']['#group'] = 'path_settings';
+
 

There should be closing brace

sure, still need re-roll

rerolled.

+++ b/core/modules/path/path.module
@@ -58,28 +56,25 @@ function path_permission() {
+function path_form_user_form_alter(&$form, $form_state) {
+  $user = $form_state['controller']->getEntity();

form state should be a class #2335659: Remove FormState ArrayAccess usage from core

I have changed it. It seems that run ok.

Now it pass this test in my local.

Looks like this needs another reroll.

However it's worth pointing out that this is currently filed as a 'Feature request' so it's more likely that this would be added to 8.1.x than 8.0.x.

Yes, really I think that is better wait for a more stable 8.0.x for do the rerolling. Ty for all Cottser. :)

Rerolled.

I'm just fixed an endline and cleaned a whitespace in #77.

path_alias_user-476294-79.patch queued for retesting.

Just changed patch from @adci_contributor #79 to make it applying current HEAD.
Also tested it: it works, but I let someone else RTBC ! ^^

Oups!
Just re-rolling patch under 8.0.x-dev !! (héhé)

The path_settings_form_element() is not actually reusable because assumes $form['path'] so maybe better to implement that function with argument of path field?

Also usage of this details could be configured on path field widget...

+++ b/core/modules/path/path.module
@@ -43,28 +43,24 @@ function path_help($route_name, RouteMatchInterface $route_match) {
 function path_form_node_form_alter(&$form, FormStateInterface $form_state) {
...
+  path_settings_form_element($form);
...
+function path_form_user_form_alter(&$form, $form_state) {
...
+    path_settings_form_element($form);
...
-  if ($entity_type->id() === 'taxonomy_term' || $entity_type->id() === 'node') {
+  if ($entity_type->id() === 'taxonomy_term' || $entity_type->id() === 'node' || $entity_type->id() === 'user') {

@@ -80,6 +76,26 @@ function path_entity_base_field_info(EntityTypeInterface $entity_type) {
+function path_settings_form_element(&$form) {
+  $form['path_settings'] = array(
+    '#type' => 'details',

A bit confusing to have details on user and node forms but no on taxonomy term

But do we really need details on user form? and not on taxonomy form?
Why don't we implement path in user the same way it was done for taxonomy, with no details.

And on approach

+    '#group' => 'advanced',

that line seems redundant, since there is no advanced group in user form unlike node form.

Thank you for the work on that.

Why do we consider doing a feature request for 8.0, that certainly sounds like 8.1 material for me.

Hi,
I'm submitting a tentative patch to harmonize the behavior of the path field inside entities.
As noted in previous comments it makes no sense for the path field to be inside details on the node form but not on taxonomy or user ones so I harmonized it and added it inside details in all forms.

Part of the Drupalcon Dublin 2016 sprints.

Resubmitting the previous patch in order to pass tests.

I will review the patch by ieguskiza in #98 on Drupalcon Dublin.

the patch is not applying
i re-rolled it

The patch applied cleanly for me.
Downloaded and enabled pathauto, token and ctools.
Set a url path for my user.
Working perfectly.

This patch is not needed for pathauto to work, as it has settings that allow to enable the field on every entity type.

I do not think having this makes sense without automatically generated aliases, so I'm kind of -1 on adding this to core.

The same could be said for taxonomy terms but I guess it was decided to add them directly through the path module because of the widespread usage we give to aliases in regards to these entities.

That being said, I agree that assuming the same usage for user entities is quite far-fetched and probably a very specific case for certain sites. Also the path field was never added for users in the core of Drupal 7 to begin with, so even though I provided a patch for this feature I will also say that we would be better of scrapping it.

Re rolled #98 as not applied.

I have done changes in "core.entity_form_display.user.user.default.yml" as the error was coming from file.

Please Review

Thanks!!

The patch is not applying on the D9.1.x branch.

Re-rolling the patch for the same.

Below is the output of test cases that are passed on local which are failed in the previous patch.

root@b1dad303faf2:/var/www/html# phpunit --configuration=core --filter testMediaForm core/modules/path/tests/src/Functional/PathMediaFormTest.php
PHPUnit 8.5.4 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\path\Functional\PathMediaFormTest
.                                                                   1 / 1 (100%)

Time: 5.75 minutes, Memory: 8.00 MB

OK (1 test, 10 assertions)

HTML output was generated



root@b1dad303faf2:/var/www/html# phpunit --configuration=core --filter testConfig core/profiles/demo_umami/tests/src/Functional/DemoUmamiProfileTest.php
PHPUnit 8.5.4 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\demo_umami\Functional\DemoUmamiProfileTest
.                                                                   1 / 1 (100%)

Time: 24.97 minutes, Memory: 8.00 MB

OK (1 test, 215 assertions)

@sharma.amitt16, I have tested by applying this patch, and the path alias functionality working fine.

Tested the patch given in #121. Patch applied successfully and changes are reflecting with custom path aliasing.

Please refer to SS for ref. RTBC+1

+++ b/core/modules/path/path.module
@@ -98,7 +99,9 @@ function path_entity_base_field_info_alter(&$fields, EntityTypeInterface $entity
+  $default_entity_types = ['taxonomy_term', 'node', 'media', 'user'];

Small nit, but since you are defining $default_entity_types twice in the module, I wonder if it makes sense to create a constant or helper function to get the array values. Then it can be well documented, and if a new entity type is added, it only has to added in one place.

Otherwise, the code looks good.

Working on it.

@PapaGrande, Thank you for review. Updating the patch as comment #125

Test case failed due to random failure, adding test again passed. changing issue status.

Successfully tested by a few people and the code looks good to me. Well done!

That logic was moved to Drupal\path\Plugin\Field\FieldWidget\PathWidget::formElement(), dynamically based on the advanced key.

This is based on very old patches and was rerolled incorrectly from that as that part has been removed.

That said, I'm still not fully convinced about doing this by default. I think aliases for user profiles is a _very_ rare use case, especially creating them manually. What is the use case for that when you don't use pathauto? And if you do, it's just a checkbox to enable it for user (like any other entity type as well).

Is this a duplicate of #1057570: Create user/edit menu path to edit the current user's account? They sound very similar but have completely different solutions.