Always automatically generate Phame slugs
ClosedPublic
Actions

Authored by epriestley on Dec 15 2015, 9:41 PM.

Details

Reviewers

chad

Maniphest Tasks

T9995: Blog posts that include a "/" character show a 404 error

Commits

Restricted Diffusion Commit
rP2d588715bc56: Always automatically generate Phame slugs

Summary

Fixes T9995. I think letting users customize slugs is not a hugely compelling as a product feature, and this fixes the issue with slugs that have "/" characters in them and makes the move to EditEngine easier since I don't have to deal with the weird JS thing.

Instead, just generate slugs automatically. No more JS, no more separate field, things automatically update if you rename a blog, and now that URIs have IDs in them the old URI will still work after a rename.

Test Plan

Applied migration.
Created new posts.
Edited existing posts.
Visited various posts.
Created a post with a bunch of "/" in the title, things still worked fine.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley updated this revision to Diff 35765.Dec 15 2015, 9:41 PM

epriestley retitled this revision from to Always automatically generate Phame slugs.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: chad.

epriestley added a task: T9995: Blog posts that include a "/" character show a 404 error.

chad accepted this revision.Dec 15 2015, 9:42 PM

chad edited edge metadata.

chad added inline comments.

src/applications/phame/storage/PhamePost.php
146

This revision is now accepted and ready to land.Dec 15 2015, 9:42 PM

Actually, I'm going to leave the column in place for now, and just remove the unique key and NOT NULL on it.

If we get rid of the data entirely, it becomes very difficult to write a "keep old URLs working mostly-OK" hack, and it's possible that we'll want to do that as an option for some installs if we have other "sleeper" installs using Phame in weird ways. Just leaving the data in the database for now doesn't cost us much and leaves that option open.