Issue 5191 - Add Terms of Use to adblockplus.org homepage

globals/has_string.py

+from jinja2 import contextfunction
+
+@contextfunction
+def has_string(context, name, page=None):

[juliandoucette, 2017/04/28 21:56:01]

Too bad get_string throws an exception :/

If we choose this solution, do you want to fix that in cms before we push this
change Jon?

[Jon Sonesen, 2017/04/29 08:30:55]

The thing is that we have it raise the exception so that if expected strings are
missing the website will generate errors letting us know so that we can see
there are problems. Additionally, it seems that you are not actually calling the
get_string global in this code. In order to access this global it would be as
follows:

context['get_string'](name, page)

This is because the globals exist outside of the pages but only under the
context object, this is due to jinja's handling of the namespaces across
template, locale, and python code contexts.

If we were to change the functionality it would be exactly the same as what you
have here with a try/except catching the specific exception rather than all
possible exceptions(as you do now, and which lead to the confusion as to whether
or not you are even calling the global you want to).

 My suggestion here would be to change the code to something like this:

try:
    context['get_string'](name, page)
    return True
except KeyError, RuntimeError:
    return False

However, I am open to discussion as to why it would be better to change this in
the CMS itself. For example, a has_string global would actually be pretty useful
in the CMS core.

[juliandoucette, 2017/05/02 08:39:17]

Ack.
Good catch! I thought that I had tested this properly... I was mistaken.
We have at least one use case:

1. When a string should not appear if it has not been translated and approved by
a manager in crowdin for legal reasons

I think this applies to all legal text.

(By default, a non-translated + non-approved string will appear in the website's
default language.)

[juliandoucette, 2017/05/02 08:45:45]

Done.

+    try:
+        if get_string(name, page):
+            return True
+    except:
+        return False