Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#650 closed defect (fixed)

CMS does not accept slash in title of job ad

Reported by: maren Assignee: sebastian
Priority: P3 Milestone:
Module: Infrastructure Keywords:
Cc: trev Blocked By:
Blocking: Platform:
Ready: no Confidential: no
Tester: Verified working: no
Review URL(s):

Description

Environment

eyeo.com

How to reproduce

  1. When creating new job ad
  2. add title with slash
  3. Look at newly created page

Observed behaviour

Site shows an error - Could not find file

Expected behaviour

Load page (new job ad) and see slash in title

Change History (11)

comment:1 Changed 5 years ago by philll

To verify this being a bug, I just wanted to reproduce the issue. Apparently, your description does not provide, where to navigate to actually conduct the described steps.

comment:2 Changed 5 years ago by trev

  • Cc trev added

The jobs.html template uses the following macro to map job titles to file names:

{% macro slugify(title) %}{{ title|lower|replace(" ", "-") }}{% endmacro %}

This should really replace all characters but \w and + into "-", I guess we'll need to add a custom filter for that (into the /filters subdirectory of the repository). Unfortunately, it won't be quite as intuitive any move - the include with the matching file name still has to be created manually.

comment:3 Changed 5 years ago by trev

While at it, we should decouple the names of the include files from the titles of the job openings - they should be specified explicitly, everything else is confusing for anyone without intimate knowledge of how this page works.

comment:4 follow-up: Changed 5 years ago by sebastian

  • Owner set to sebastian

I have fixed it for now by replacing "/" in addition to spaces with "-":
https://hg.adblockplus.org/web.eyeo.com/rev/5863353ae87f

This might not be perfect. But I find it rather intuitive to have the filenames based on the job title. Also this ensures human-readable URLs. And by auto-generating the filename we avoid redundancy.

comment:5 Changed 5 years ago by sebastian

Using a template filter with the regex suggested above ([^\w+] -> "-") now:
https://hg.adblockplus.org/web.eyeo.com/rev/51219d36e1c8
https://hg.adblockplus.org/web.eyeo.com/rev/eda6a58b2196

comment:6 Changed 5 years ago by sebastian

  • Resolution set to fixed
  • Status changed from new to closed

comment:7 in reply to: ↑ 4 ; follow-up: Changed 5 years ago by trev

Replying to sebastian:

This might not be perfect. But I find it rather intuitive to have the filenames based on the job title. Also this ensures human-readable URLs. And by auto-generating the filename we avoid redundancy.

I strongly disagree, there is nothing intuitive about how the file names are being generated now. In particular, you cannot expect an HR person to perform the necessary manipulations. Note that this has nothing to do with the URLs, these can still be autogenerated - just not the include file names. I made sure that these are specified explicitly now:

https://hg.adblockplus.org/web.eyeo.com/rev/06d3f55c0d69

I also made sure that the slugify filter doesn't generate multiple subsequent dashes, it should be growth-hacker-data-analyst rather than growth-hacker---data-analyst:

https://hg.adblockplus.org/web.eyeo.com/rev/9c2ebbfbe5e2

comment:8 in reply to: ↑ 7 ; follow-up: Changed 5 years ago by sebastian

Replying to trev:

I strongly disagree, there is nothing intuitive about how the file names are being generated now. In particular, you cannot expect an HR person to perform the necessary manipulations.

In particular with that in mind, I rather tried to keep the amount of parameters that needs to be manually configured with source code as small as possible.

Note that this has nothing to do with the URLs, these can still be autogenerated - just not the include file names. I made sure that these are specified explicitly now

Sure, that is an option.

I also made sure that the slugify filter doesn't generate multiple subsequent dashes, it should be growth-hacker-data-analyst rather than growth-hacker---data-analyst

Yeah, that is a good idea.

comment:9 in reply to: ↑ 8 Changed 5 years ago by trev

Replying to sebastian:

In particular with that in mind, I rather tried to keep the amount of parameters that needs to be manually configured with source code as small as possible.

Yes, ideally we would remove that list and simply have it generated from the list of include files (titles could be defined as a property at the beginning of the file). That's not quite trivial however and will likely require a change of the CMS, never mind not being able to specify the ordering. Until then explicitly listing the file names should be a more stable approach - a quick change of the title won't immediately turn into a disaster.

comment:10 Changed 5 years ago by fhd

  • Priority changed from Unknown to P3

Seems like a P3 to me, it makes a job ad we have live right now look bad.

comment:11 Changed 5 years ago by fhd

Ah, was already fixed. Well, now we all know it was a P3 :)

Note: See TracTickets for help on using tickets.