New URL-Aliases admin UI

Here's a screen shot of edit interface.

Just noticed, chx has updated his issue with 'only' the interface for alias editing, which i assume would be done "better". Maybe I should follow with this patch over there. Will talk to chx about it. Btw, this patch includes the last patch at: http://drupal.org/node/141526

Still would be good if someone reviews the patch, it'll help :)

Moving to: http://drupal.org/node/106094

Back to this issue as per chx's suggestion, because the above issue discusses some unrelated stuff, like tab or pop-up.

Please review..

Did you address any/all of my comments in #62 from the previous issue?

Yes, all. Please review, there were some more features to the interface like a "Delete all" button, to delete all aliases for a system path in certain language.

The system.install update number needs to be incremented.

But when I manually fix that error and try to add a path alias via node/X/edit the alias is never saved.

Very nice functionality, especially needed for proper SEO.

CODE REVIEW
- i think + if (isset($_POST['op']) && ($_POST['op'] == t('Delete all'))) can be removed in favor of adding a handler just to the delete all button. thats new for fapi3.
- why are we implementing hook_user and hook_comment? does something autocreate aliases for every user and comment? i am missing something here.
- "Optionally specify an alternative URL by which this node can be accessed.". I would remove the word 'alternative', given the fine redirect that this patch does.

This one applies to new HEAD revision.

node/X/edit actually sets the alias, but when editing it shows only active alias. so the alias which you wasn't set as active. With this patch, alias set at node/X/edit will be set as active.

Also, I would rather not include all of filter.module this early in bootstrap. did you do that just because we need filter_xss_bad_protocal? if so, consider moving that function to bootstrap.inc instead.

A total of 5 functions were needed from filter.module, so rather than moving to path.inc or bootstrap.inc, we just included filter.module. The patch now uses submit handler for Delete all button, the fapi3 way. Also fixed deleting path alias by path ID. hook_user, hook_comment, and hook_nodeapi, are used to delete aliases assigned to comment, user or node path, when they are deleted from database(op=delete), because the comment or user no longer exists, it's of no use keeping the aliases for them.

Sorry, here is the patch.

- i can only find one minor UI glitch. I see Alias 1 repeated even for the 2nd alias listed for a given node. I actually suggest getting rid of these titles. the system url and new alias titles are helpful, but the numbered ones are not IMO.
- "$_GET['destination']) ? $_GET['destination'] : 'admin/build/path'". can you just try admin/build/path' here. i think the form submission code wil just use destination if it is on the URL. no need to handle it.

• Fixed user access to path edit form.
• Removed Alias 1.. Alias 2..., prefix from alias fields.
• Fixed the destination code as suggested above. When new alias is added, user is redirected to edit form for that alias so the he/she may add more, etc, otherwise redirect to overview page.
• Fixed some bugs with filter form.
• Slight text changes.

Sorry, above patch included some jquery.js stuff. Use this instead.

Works for me.

I haven't looked at the code yet -- I started by looking at the screenshot.

Essentially, I'm confused by the Language drop down menu at the bottom. It's not clear what the current workflow is like, or what would happen if I changed the language drop-down menu. Would that cause duplicate path aliases to be inserted?

My suggestion #1: shouldn't there be a drop down menu next to each alias' textfield? That would provide me with a better overview of all the aliases, and it would allow me to translate aliases faster.

My suggestion #2: shouldn't there be a mechanism to remove a path alias from that list?

My suggestion #3: I'd use this interface on the 'add path alias' page but also in a fieldset in the 'edit node' page.

How the language thing works:
# User chooses a language(in select list) on add alias form. Proper validation is done, if the alias exists in that language. If not, the alias is added to that language.
# Edit screen is by language, i.e. if user is editing aliases for german, he won't see aliases for 'all' or any other languages. On aliases overview table, the edit link under Operations links to ?q=admin/build/path/edit/all/node/1, where 'all' is language, and the rest part is the system path. say the same for german would be ?q=admin/build/path/edit/de/node/1. The language select list's attribute is set to disabled as here it's just for user's information that he's editing alias for foo.

A language select list per alias would really be causing hick ups at validation, or at least it would take lots of code changes, making it hard to review.. :P

# To remove an alias, clear the alias from field and submit. A help text stating that should go in.

# On node edit page, only the active alias is shown and can be set currently. In whole table on that page, user permissions, as currently set, won't let the node owner edit the aliases, he can only add new if he has 'create new aliases' permission. Is it worth displaying the whole table?

I still think there is room for UI improvements here. Putting all translations on the same UI really helps when you're the single translator (like many people in Belgium are).

Marking this 'code needs work' until some of these suggestions have been tested/implemented. Thanks.

This patch is just a re-roll. As discussed on IRC, the interface showing language drop downs for each alias field didn't prove well at end user's understand factor. Here's the screenshot: http://img2.freeimagehosting.net/uploads/d9775a8a5a.png It's just confusing for one who just started with these things.

So, chx suggested to have tabs for each language, like an "English" tab would appear on top, clicking which shows aliases for that language, default tab being 'All languages'(all is not every language). It's easy to accomplish this with the new menu system, but in that, we could fetch only the first part(i.e. before a slash), of the source path. i.e. suppose we are on this page ?q=admin/build/path/edit/all/forum/1 and our menu tab item in hook_menu for english language is: admin/build/path/edit/en/%
so that wildcard is only a keyword within slashes, whereas we need all the part of url after that. so that tab would link us to: admin/build/path/edit/en/forum

the reason for explaining this is if there's anyone with an idea for the interface when we are dealing with multilingual aliases? could a feature request be made to menu system to allow a new wildcard which is placed at end of path and it passes the rest part of url to the callback?

only new thing with this patch is, delete operations link in edit path interface, and removed "delete all" button.

Note: I don't really know how languages work in general in Drupal 6, so this may not make total sense in context. However...

- Languages are all on one page.

- There is a drop down menu next to all the aliases allowing easy selection of language

- It is only possible to select one permanent/default/active URL per language. In other words there will be no error message since it is impossible to enter more than one default per language.

- Aliases and permanent URLs are separated to make it clear they are different.

- Easy to delete aliases (not necessarily a good thing, but hey)

- No need to validate user input for alias language. The drop-down list, next to redirects, only contain languages that have a permanent alias associated with them already.

This one implements using tabs, and works against current HEAD revision. Screen shot coming ahead.

the screenie.

would be nice to get this one in D6.

I really like both the concept and the UI (tabs version). Would be good to get it in.

Love it, but patch doesn't apply :x

fresh 6.x tarball, patched it, installed.

Cannot redeclare system_update_6024() (needs to be 6025)

fixed that manually in system install, patched, installed.

I created two nodes. Added path alias on node add form for each - this was fine.

Went to path admin, added an extra path, worked fine.
Redirect from non-active to active alias is fine. And I love the way it finds existing paths when you add a new one.
Added a third path - WSOD.

admin/build/path/add works
admin/build/path/edit/all/4?destination=admin%2Fbuild%2Fpath - WSOD when I save any changes at all - either a new alias or changing the "redirect to" radio.

Everything about it is great apart from these bugs. Didn't test any of the multi-language stuff at all.

Which of the following interface is better, and simple to understand(for novice starter)?
Screenshot 1
Screenshot 2
Screenshot 3

IMO, 1st one is as simple as it could be, and the basic one.
2nd one too much complicated, although it allows quick and advanced users to complete their task as quickly as possible.
3rd is same as the 1st one, just each language's tab appears at top allowing the user to set aliases for each language under each tab.

so... which one do you vote for? maybe we can get this into core as soon as possible, before there's the next feature freeze.

#3 look good to me. glad we are still working on this.

I like #3.

Should expand. Last time I tested the patch, #3 was the interface, and it was very, very easy to use and behaved exactly how I'd expect. Massive, massive improvement on the current interface.

Very much hopefully this patch covers most of tiny tod bugs, which were caught while testing this time. and hopefully we get this in soon :)

Review? :'(

Fresh Drupal 6.x-dev tarball. Installed first, patch applied cleanly. System update ran with no problems, didn't generate any errors at all while I was going through.

The user interface is a massive improvement on the existing one, I'd go so far as to say it's ergonomic - adding multiple aliases becomes very, very easy, because you immediately see all aliases for a path, so you get an overview of what's there. Also the default radio selections make loads of sense and save some mouse clicks in most situations.

This is considerably better than having to go through over 500 pages of aliases on my current site. There's no way, to find all aliases related to one system path at the moment without looking through manually (the new filter only applies to aliases themselves).

Also, the active path functionality works fine, is 100% necessary for these UI improvements, and also sits well with possible path lookup optimisations elsewhere in the queue (splitting out of 'object' paths etc.).

Sorry the above patch included outdated .htaccess changes. And screen shot, of what we have without the locale module, to follow in next post.

Screen shot of interface, with locale module disabled.

First of all, nice patch!

I tested the patch on latest HEAD and this really make things straight forward. I cannot come up with any improvements about coding style, (error) messages or any functionality this patch offers that isn't the way I expected it to be.

So, I'm setting this RTBC..

Very nice!

Fixed a minor bug in hook_nodeapi. Language was not being set for aliases when inserting or updating.

Thanks for the review Stefan!!

* I like screen #2 best. It makes it faster to manage multi-lingual content. Belgian translators have asked me for _exactly_ this UI.

* Looking at the screenshot, the page title looks wrong. It should probably be 'Edit $path' or something.

* Patch no longer applies.

Asking people what UI they want is never going to lead to something they actually do want. Most people are not UI designers. I presume what they actually want is an easy to grok, but fast and efficient interface. They are not going to get that combination with any of these options.

Option 2 would be a compromise with a bias towards speed of operation. But it is completely non-intuitive, so you are serving up another learning curve with this. Make sure you are aware of the dichotomy here.

Can we run the Advanced Path alias edit interface as a contrib :P
i was confused using that interface too. if you really have more thumbs for that interface, then will update the patch as soon as possible. Need feedback from others please.......

I still like #2 for the interface. Don't like the idea of checkboxes in a single language situation for a start (assuming they don't revert to radios if only one language).

Waiting for http://drupal.org/node/157510 to get committed.

The patch this issue was hold off on got committed, so this might be the time to review this issue. (Not for Drupal 6 unfortunately).

OK, it was sad we missed D. But lets get this into D7? Anyone up for refreshing this?

I'll submit the active alias feature as a separate patch, and only after improved interface patch gets in.

subscribe

Wish I figured out where its going kaput, but this isn't working out. On patching a clean HEAD, it isn't allowing me to edit any alias or add an alias and gives a 'Page not found' error. Still, going over the code...