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.

Sign up for GitHub

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

Automatically remove any trailing slashes #726

Merged
merged 9 commits into from
Apr 26, 2020
Merged

Automatically remove any trailing slashes #726

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9a1737f
Automatically remove any trailing slashes
tunetheweb Apr 12, 2020
2721e76
Handle trailing slashes on odd pages
tunetheweb Apr 12, 2020
5e4c0c1
Merge branch 'master' into trailing_slashes
tunetheweb Apr 12, 2020
bb4f3bb
Better comments
tunetheweb Apr 12, 2020
e53a9d7
Remove sitemap.xml redirect as not required
tunetheweb Apr 13, 2020
75c4d18
Move accessibility check to route
tunetheweb Apr 22, 2020
0daeb53
Better way of handling accessibility statement
tunetheweb Apr 26, 2020
fe491e1
Remove old accessibillity route
tunetheweb Apr 26, 2020
4f8df36
Query strings for Accessibility Statement
tunetheweb Apr 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions src/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,11 @@ def add_header(response):
def render_template(template, *args, **kwargs):
# If the year has already been set (e.g. for error pages) then use that
# Otherwise the requested year, otherwise the default year
year = kwargs.get('year',request.view_args.get('year', DEFAULT_YEAR))

supported_languages = SUPPORTED_LANGUAGES.get(year, (DEFAULT_LANGUAGE,))
year = kwargs.get('year', request.view_args.get('year', DEFAULT_YEAR))

# If the lang has already been set (e.g. for error pages) then use that
# Otherwise the requested lang, otherwise the default lang
lang = kwargs.get('lang',request.view_args.get('lang', DEFAULT_LANGUAGE.lang_code))

lang = kwargs.get('lang', request.view_args.get('lang', DEFAULT_LANGUAGE.lang_code))
language = get_language(lang)
langcode_length = len(lang) + 1 # Probably always 2-character language codes but who knows!

Expand All @@ -59,6 +57,7 @@ def render_template(template, *args, **kwargs):

# Although a langauge may be enabled, all templates may not have been translated yet
# So check if each language exists and only return languages for templates that do exist
supported_languages = SUPPORTED_LANGUAGES.get(year, (DEFAULT_LANGUAGE,))
template_supported_languages = []
for l in supported_languages:
langTemplate = 'templates/%s/%s' % (l.lang_code, template[langcode_length:])
Expand Down Expand Up @@ -161,8 +160,14 @@ def accessibility_statement(lang):
return render_template('%s/2019/accessibility_statement.html' % (lang))


@app.route('/sitemap.xml')
# Handle trailing slashes for accessibility statement (as it's a special case)
@app.route('/<lang>/accessibility-statement/')
Copy link
Member

@rviscomi rviscomi Apr 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should be able to decorate the existing accessibility_statement function with this @app.route and decide wether to redirect there based on trailing slash.

Copy link
Member Author

@tunetheweb tunetheweb Apr 23, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rviscomi I cannot get this to work.

I can get it to work the other way around. So if the route is setup as /<lang>/accessibility-statement/ then flask automatically redirects any request for /en/accessibility-statement (without the last slash) to include the slash for you with a 308 redirect. It also serves /en/accessibility-statement/ just fine without any redirect.

This is also explained in the Flask documentation: https://flask.palletsprojects.com/en/1.1.x/quickstart/#unique-urls-redirection-behavior

However our URLs have no trailing slash (except for home pages like /en/2091/) so it's /en/accessibility-statement or /en/2019/methodoloy. As we've been launched for a while now, and linked from many places, I don't think it's worth changing our URLs now (even with a 301 redirect) to include the slash, as Flask seems to prefer.

I can only get this to redirect requests for /en/accessibility-statement/ to /en/accessibility-statement in one of two ways:

  1. the have two routes like I have currently.
  2. have the validate function catch this and redirect, like I had previously (which I actually think I prefer to be honest).

I tried a few other things, including using a regex to try to catch both URLs but those are the only two things I could get to work.

@rviscomi to you want to give it a go and see if I'm missing something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK finally resolved this @rviscomi
Can you check if happy now?

@validate
def accessibility_statement_with_trailing_slash(lang):
return redirect("/%s/accessibility-statement" % (lang)), 301


@app.route('/sitemap.xml')
def sitemap():
xml = render_template('sitemap.xml')
resp = app.make_response(xml)
Expand Down
8 changes: 5 additions & 3 deletions src/validate.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,13 @@ def decorated_function(*args, **kwargs):


def validate_chapter(chapter,year):

chapters_for_year = SUPPORTED_CHAPTERS.get(year)
if chapter not in chapters_for_year:
if chapter in TYPO_CHAPTERS:
if (chapter[-1] == "/"):
# Automatically remove any trailing slashes
return chapter[:-1]
elif chapter in TYPO_CHAPTERS:
# Automatically redirect for configured typos
logging.debug('Typo chapter requested: %s, redirecting to %s' % (chapter, TYPO_CHAPTERS.get(chapter)))
return TYPO_CHAPTERS.get(chapter)
else:
Expand All @@ -63,7 +66,6 @@ def validate_chapter(chapter,year):


def validate_lang_and_year(lang, year):

if year is None:
logging.debug('Defaulting the year to: %s' % year)
year = DEFAULT_YEAR
Expand Down