416479 - [ForumUX] Pretty URLs for forums

Status: VERIFIED FIXED

(support.mozilla.org :: Forum, task)

Description

[Jason Barnabe (np)]

Among the things we could do:

http://support.mozilla.com/tiki-view_forum.php?forumId=1 -> http://support.mozilla.com/forums/support

http://support.mozilla.com/tiki-view_forum.php?forumId=3 -> http://support.mozilla.com/forums/contributors

http://support.mozilla.com/tiki-view_forum_thread.php?comments_parentId=1480&forumId=1 ->
http://support.mozilla.com/forums/support/1480

Comment 1

[David Tenser [:djst]]

Not sure if hardcoding a string (e.g. "support") to a forumId (e.g. 1) is a good idea if we want to support l10n in the future. Maybe that would be possible anyway though, by making sure e.g. /ja/forums/support points to a different forumId than /en-US/forums/support?

Even with numbers only, http://support.mozilla.com/forums/1/1480 is way cleaner than http://support.mozilla.com/tiki-view_forum_thread.php?comments_parentId=1480&forumId=1

Comment 2

[David Tenser [:djst]]

This would also make it possible to gather locale specific site stats for future localized forums.

Comment 3

[[:Cww]]

Ok, now that we have a bot to link people to forum posts in #sumo, people are complaining because the forum links are so long and wrapping on their IRC client.

This is really doable with an htaccess change, so let's just finalize exactly what we want (personally, I like the /en-US/forums/support notation rather than the /1/ notation) and then I can just write the few lines of htaccess needed for it to be done.

I'd also like /forums/new to link to the appropriate place.

Comment 4

[[:Cww]]

Sigh, I did say I would do this...

Ok:

RewriteRule ^forum/(.+)$ http://support.mozilla.com/%{BROWSER_ACCEPT_LANGUAGE}/

(I'm not completely sure about that one so someone who knows how to deal with browser languages in htaccess should probably help)

RewriteRule ^([^/]+)/forum/support/[0-9]+$ http://support.mozilla.com/tiki-view_forum_thread.php?locale=$1&forumId=1&comments_parentId=$2 [NC,L]

RewriteRule ^([^/]+)/forum/contributors/[0-9]+$ http://support.mozilla.com/tiki-view_forum_thread.php?locale=$1&forumId=3&comments_parentId=$2 [NC,L]

RewriteRule ^([^/]+)/forum/off-topic/[0-9]+$ http://support.mozilla.com/tiki-view_forum_thread.php?locale=$1&forumId=4&comments_parentId=$2 [NC,L]

This would turn http://support.mozilla.com/en-US/forum/support/1480 into http://support.mozilla.com/tiki-view_forum_thread.php?locale=en-US&comments_parentId=1480&forumId=1

Doing the reverse is a little tricky (that is turning http://support.mozilla.com/tiki-view_forum_thread.php?comments_parentId=1480&forumId=1 into http://support.mozilla.com/en-US/forum/support/1480 so that the link is more bookmarkable and tracked by omniture, I'll work on it)

Comment 5

[[:Cww]]

Sigh, I overwrote something screwy in my first one:

RewriteRule ^forum/(.+)$ http://support.mozilla.com/%{BROWSER_ACCEPT_LANGUAGE}/forum/$1 [NC]

Comment 6

[[:Cww]]

Looking at .htaccess on sumo_tools, I probably have to put a lot more thought into not overwriting /forum/ and the like.  I'll squint at it some, and test lots and come back with a proper patch.

Comment 7

[David Tenser [:djst]]

Don't sweat it, Cheng. You have more important things to work on. ;)

Comment 8

[David Tenser [:djst]]

My proposed URL "design" would be http://support.mozilla.com/%locale%/forum/x/yyyy, where x is the forum id, and yyyy is the thread id.

Comment 9

[Paul Craciunoiu [:paulc]]

So I can do this one way -- making sure urls of type http://support.mozilla.com/%locale%/forum/x/yyyy work properly. However, I'd like to rewrite the "ugly" urls to be of this form now. Anyone knows how to do that?

E.g. I want 
tiki-view_forum_thread?locale=en-US&forumId=1&comments_parentId=1807
to go to
/%locale%/forum/x/yyyy

The problem I hit on when I tried to do this was looping between this rule and the one that goes the other way.

Comment 10

[Paul Craciunoiu [:paulc]]

Attachment: patch, v1

This adds pretty URLs and rewrite some of the relative paths to images.

Comment 11

[Paul Craciunoiu [:paulc]]

Attachment: SQL to add regexes for forum homepage and forum topic listing

This SQL adds two regexes that handle the rewrites for these two pages:
* tiki-view_forum.php
* forum home page (Popular + Recent topics)

We may add subsequent regexes if I missed any listings of forum topics.

Comment 12

[Paul Craciunoiu [:paulc]]

r49035 on forumux branch. We'll need to run htaccess.sh and the SQL above. Waiting on Laura's review before filing an IT bug.

Comment 13

[Laura Thomson :laura]

Comment on attachment 392784 [details] [diff] [review]
SQL to add regexes for forum homepage and forum topic listing

Bit of tidying needed in the second regex as discussed in IRC.

Comment 14

[Laura Thomson :laura]

Waiting on revised regex.

Comment 15

[Paul Craciunoiu [:paulc]]

Attachment: updated SQL to prettify URLs

Cleaned up that regex. Also added rewrites for forum URLs (e.g. when viewing forums, not threads).

While I was looking at my dump, I noticed we have a regex that looks like this:
It takes:
^(.+)\?$
to
$1

Am I missing something, why is this useful?

Comment 16

[Paul Craciunoiu [:paulc]]

Attachment: patch, v3

This updates the code by adding a pref that helps keep the notification library productizable if URL rewrites are not set. It also adds {$tikiroot} in most places to help with the absolute/relative URLs

Finally (and notably), posting a new thread now redirects to the friendly URL (depending on the pref). Reporting a post also. So hopefully users will stay in the friendly realm 100%! :)

Comment 17

[James Socol [:jsocol, :james]]

Comment on attachment 397763 [details] [diff] [review]
patch, v3

I ran into some conflicts applying this. Generating a new patch now.

Comment 18

[James Socol [:jsocol, :james]]

Attachment: patch, v3.1

Conflicts resolved

Comment 19

[James Socol [:jsocol, :james]]

r51762 / r51764

Remember to rerun webroot/htaccess.sh after svn up to grab the rewrite changes.

Comment 20

[Laura Thomson :laura]

We'll need that run on stage too.

Comment 21

[Vishal Kamdar [:vish_moz]]

Verified, FIXED.
Found 4 other bugs related to this one:  Bug 518195, Bug 518223,  Bug 518209,  Bug 518228

Comment 22

[[:Cww]]

The plan is to take this patch as is because none of the bits where pretty urls are missing are critical but to take future patches where we fix them as we find them.