Page MenuHomePhabricator

Blog posts that include a "/" character show a 404 error
Closed, ResolvedPublic

Description

Hi Team;

After updating the code to latest master version on December 13th blog posts (new and old) that contain a "/" character in the title display a 404 error with the following page.

Screen Shot 2015-12-15 at 4.22.00 PM.png (480×1 px, 37 KB)

Posts without that character on the title display just fine.
The final URL is something like this:

http://phab.example.com/phame/post/view/775/weekly_updates_12/11/

I'm not sure what the URL looked like before but it worked just fine for more than a year or so.
Updating to latest master code on Dec 15th didn't resolve the issue.

**This is a regression.

Let me know if I can provide any more details.

Thanks!

Event Timeline

nicolast raised the priority of this task from to Needs Triage.
nicolast updated the task description. (Show Details)
nicolast changed the edit policy from "All Users" to "Custom Policy".
nicolast added projects: Bug Report, Phame.
nicolast added subscribers: chad, Taskle.
nicolast added a subscriber: nicolast.

You'll need to edit your title phame_title to fix the 404 for now (I don't think it's worth providing a retroactive fix). We should have always been removing / from Phame Title as it would have always 404'd with Live View.

@chad Hi Chad, so how can we remove it if we can't edit the post? Is there a workaround to edit the post? Or some manual database change we can make? We have literally hundreds of blog posts as we use this feature internally to track weekly work (hence the urgency as now our entire company has lost visibility into each other's work), so it would be a nightmare to do manually...

Thanks!

You can use the URL /phame/post/edit/775/

I don't know how involved a migration script would be, @epriestley?

(maybe we can hack the route?)

I am stoked someone uses Phame though!

As a workaround, you can apply this patch locally, which should make things work for now:

diff --git a/src/applications/phame/application/PhabricatorPhameApplication.php b/src/applications/phame/application/PhabricatorPhameApplication.php
index 1a49c95..33df127 100644
--- a/src/applications/phame/application/PhabricatorPhameApplication.php
+++ b/src/applications/phame/application/PhabricatorPhameApplication.php
@@ -48,7 +48,7 @@ final class PhabricatorPhameApplication extends PhabricatorApplication {
           'blogger/(?P<bloggername>[\w\.-_]+)/' => 'PhamePostListController',
           'edit/(?:(?P<id>[^/]+)/)?' => 'PhamePostEditController',
           'history/(?P<id>\d+)/' => 'PhamePostHistoryController',
-          'view/(?P<id>\d+)/(?:(?P<slug>[^/]+)/)?' => 'PhamePostViewController',
+          'view/(?P<id>\d+)/(?:(?P<slug>.*)/)?' => 'PhamePostViewController',
           '(?P<action>publish|unpublish)/(?P<id>\d+)/'
             => 'PhamePostPublishController',
           'preview/(?P<id>\d+)/' => 'PhamePostPreviewController',
@@ -92,7 +92,7 @@ final class PhabricatorPhameApplication extends PhabricatorApplication {
     return array(
       '/' => array(
         '' => 'PhameBlogViewController',
-        'post/(?P<id>\d+)/(?:(?P<slug>[^/]+)/)?' => 'PhamePostViewController',
+        'post/(?P<id>\d+)/(?:(?P<slug>.*)/)?' => 'PhamePostViewController',
       ),
     );
   }

We can migrate the slugs pretty easily.

We could also just bring that patch upstream, but I think allowing "/" in slugs is probably a decision we'd regret in the long term.

Yeah it's a critical part of our workflow :) back at Google there's this concept of "weekly snippets" that the whole company used to use and our engineering team here at Doctor.com models after that. It's really useful to get visibility into work. :)

So it looks like this workaround works for some of the posts... Note that some of our phame titles have / in them, e.g.
http://dev.doctor.com/phame/post/view/781/12/18/15/
If I rewrite to
http://dev.doctor.com/phame/post/edit/781
I see that it then saves properly.

But yes, it would amazing if you could provide any way to migrate old data.

(the reason this issue is exacerbated for us is because the titles of all our posts are dates, which many folks use /'s for, so we have a ton of posts with this issue)

Also, is the fact that the URL scheme changed an issue (i.e., do you have a lot of links to the old URLs in external systems)? We weren't aware anyone was really using Phame seriously, but I can write a little extension you could use to keep the old redirects working for a while if that's also causing you pain.

(This is somewhat beyond the scope of our normal support for prototype applications, but your use case is generally reasonable and Phame is kind of an unusual case.)

We don't have external references to Phame, only internal references within Phame itself. So no need to have the old urls work in that regard. It's just that clicking the links in the phame UI itself is an important piece we'll need to work.

As an aside, I'm always impressed by your support for a free tool! :)

Since slugs no longer need to be unique, I'm just going to simplify this whole process and generate them automatically from titles.

We could let you customize the slug again some day, but I suspect this feature is nearly unused, not hugely valuable in cases where it is, and definitely not worth its weight in terms of complexity or prominence on the edit form.

Awesome :) yes, I don't see any of us caring about the slugs. In fact I remember it being a bit of a nuisance to do this because we'd have 15 people all with the slug of, for example, 12/10/2016, so we'd all have to make them unique. :)

So yes, that would be the best of all possible worlds!

As an aside, if you guys ever want insight from heavy Phab users, let me and Nico know! We're so deeply integrated now...we even automate the creation of Phab tasks via Salesforce to manage our entire Client Services team's work flow to onboard new clients. :)

Well we're about to unbeta Phame if you have any use cases we're not thinking of.

Sounds like you're using it as we do, though.

The only feedback I have is that since the "New Blog" button was added top Right (instead of, for example, "New Post" which would default to, say, the last blog that you posted to), I inadvertently created a new Blog when I meant to write a post.

That may sound silly but I think it would be great to have a single-click way from the Phame homepage to create a new blog, since it's the primary use case, IMHO.

Other than that and this issue, I think it's great and the changes you guys have made have all been great!

The + is a link to create new posts, may not be 100% obvious.

pasted_file (353×320 px, 35 KB)

Oh cool - that's a useful shortcut. But imagine that the primary use case is this sort of note taking where everyone has their personal blog, I anticipate that 95% of the time, people will be writing blog posts on the same blog they last wrote on. For the 15 people on our team, 100% of us only write on one blog which is our personal blog.

This is just a nice-to-have shortcut, though. I'll start clicking there... this won't be as easy as when we have 30 engineers and I need to navigate to the right place on the left hand nav, but again this is all just nice-to-have stuff in the grand scheme of things.

Ah ok, I don't think we've considered the "everyone has a blog" case. It might get hairy.

Even if it's not an everyone has a blog case, again, I can't imagine it's a common use case to regularly be switching blogs to post on. Again, this is just a convenience function (a "New Post" button that creates a new post for the last-posted-to-blog). Not required, just a nice to have.

This should be fixed in HEAD. Let us know if you're still seeing issues or we missed anything.

(I've deployed it to this server and things seem to be working OK for us, at least.)