Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an optional category-specific [slug] for email subject lines. #4799

Closed
wants to merge 1 commit into from
Closed

Add an optional category-specific [slug] for email subject lines. #4799

wants to merge 1 commit into from

Conversation

icculus
Copy link
Contributor

@icculus icculus commented Apr 11, 2017

This lets us turn "[Game Development] actual subject..." emails into
"[GAMEDEV] actual subject" without resorting to making the official name of
the category ugly (or breaking URLs and/or uglifying the usual category slug). We need this as we add more things that were mailing lists, so Mailing List Mode people continue on with the subject slug people expected.

A lot of this was built by looking at revision control to see what had to be touched for other fields in the Category Settings UI, but I was basically stumbling around in the dark, so if something looks obviously wrong, it probably is.

This needs a field added to the Categories table, so there's a db/migrate entry. The UI needs a new string translated; I filled in English only.

@mention-bot
Copy link

Thanks @rcgordon for your pull request 👍. By analyzing the blame information on this pull request, I identified @ZogStriP and @eviltrout to be potential reviewers.

@discoursebot
Copy link

You've signed the CLA, rcgordon. Thank you! This pull request is ready for review.

@icculus
Copy link
Contributor Author

icculus commented Apr 11, 2017

(Just rebasing in case it fixes the Travis CI failure.)

@icculus
Copy link
Contributor Author

icculus commented Apr 11, 2017

Also, if you want to see this in action, I set this up on a temporary Droplet at http://discourse.icculus.org/ ...make an account and I'll give it admin and you can play with it. I'll destroy the droplet when this PR is closed.

@icculus
Copy link
Contributor Author

icculus commented Apr 12, 2017

Ok, I literally have no idea why Travis CI is failing now. :/

@tgxworld
Copy link
Contributor

Hi @rcgordon :) Thank you for contributing. Has this feature been discussed on meta.discourse.org? If not, you probably want to open a topic on meta so that we can decide if this feature should go into core.

@icculus
Copy link
Contributor Author

icculus commented Apr 13, 2017

Has this feature been discussed on meta.discourse.org?

I've mentioned it on meta--not as a full topic--and I've seen a comment or two elsewhere saying %{optional_cat} is really long for a subject tag, can we get it to use the category slug instead?" and this is a further customization of that concept.

I can start a new topic about it, but it feels like the sort of patch that will get bikeshedded into defeat. If this is a hard requirement to accepting a pull request, I will do it, but this patch adds a reasonable feature in not many lines of code, so I'm happy if the Discourse team wants to judge it directly, too.

@tgxworld
Copy link
Contributor

@coding-horror @SamSaffron Any thoughts about this PR?

This lets us turn "[Game Development] actual subject..." emails into
"[GAMEDEV] actual subject" without resorting to making the official name of
the category ugly (or breaking URLs and/or uglifying the usual category slug).
@icculus
Copy link
Contributor Author

icculus commented May 2, 2017

Merged PR with the latest and greatest to remove conflicts.

@coding-horror @SamSaffron: do you have an opinion on this PR's concept? I'd like to shut down the test Droplet I've got for this one way or the other, so a "this could be useful" or "this is a terrible idea in general" would be appreciated when you have a moment.

Thanks!

@SamSaffron
Copy link
Member

For this particular case, why would you want a different one slug in the URL and another in the email.

Does a simple site setting of: prefer category slugs in emails suffice here? Or simply allow you to specify the format of the "emails title" so you can place stuff wherever?

@icculus
Copy link
Contributor Author

icculus commented May 2, 2017

Does a simple site setting of: prefer category slugs in emails suffice here?

We'd prefer not to use the category slugs for a few reasons, some better than others:

  • We have existing mailing lists we are migrating, and many of those users will be using Discourse exclusively in Mailing List Mode, beyond creating accounts in the web UI. They will want mail to continue to have the slugs they expect (both for user comfort and for scripts that might toss incoming mail into subfolders based on subject slugs), which are...
  • ...different than the category slugs we've been using for the past several weeks, and we don't want existing links on the web to URLs that have those slugs to break needlessly.
  • The existing category slugs are pretty and descriptive ("game-development") as they should be for a URL, but not terse ("[GAMEDEV]") as they should be for an email subject line where pixel real estate is at a premium.

Or simply allow you to specify the format of the "emails title" so you can place stuff wherever?

I'm not married to my solution specifically; I went this route because there's already a way to customize email subjects in the settings, with a handful of Discourse-supplied variables, so I added one more variable. But any solution that gets me there is fine with me. I don't know the specifics of "emails title" you mentioned vs the "email subject" field in Settings, but I'll work towards whatever you think is best.

@icculus
Copy link
Contributor Author

icculus commented May 20, 2017

There doesn't seem to be a lot of enthusiasm for this patch, so I'm just going to change my category slugs and use those for email subject slugs, too. :/

If y'all want to get this into revision control at some point, I'll take the time to figure out what's happening with the failing Travis-CI check, otherwise, you can just close this PR, no hard feelings.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants