In this case using `joins` doesn't prevent N+1 queries to get titles for
every record, and since we cannot order translations with just SQL due
to fallbacks, we don't need it.
Automatic SQL injection checks were showing a false positive in this
scope; there was no real vulnerability here because foreign keys, table
names and locales were under our control.
Sanitizing descriptions before saving a record has a few drawbacks:
1. It makes the application rely on data being safe in the database. If
somehow dangerous data enters the database, the application will be
vulnerable to XSS attacks
2. It makes the code complicated
3. It isn't backwards compatible; if we decide to disallow a certain
HTML tag in the future, we'd need to sanitize existing data.
On the other hand, sanitizing the data in the view means we don't need
to triple-check dangerous HTML has already been stripped when we see the
method `auto_link_already_sanitized_html`, since now every time we use
it we sanitize the text in the same line we call this method.
We could also sanitize the data twice, both when saving to the database
and when displaying values in the view. However, doing so wouldn't make
the application safer, since we sanitize text introduced through
textarea fields but we don't sanitize text introduced through input
fields.
Finally, we could also overwrite the `description` method so it
sanitizes the text. But we're already introducing Globalize which
overwrites that method, and overwriting it again is a bit too confusing
in my humble opinion. It can also lead to hard-to-debug behaviour.
Now we take into consideration locales persisted but marked for
destruction to complete some logic and to be able to show best
translations on different situations.
In order to not allow users to remove all persited
translations from any resource. A few exceptions were
added:
* Does not apply to globalizable models without
translatable attributes required
* Make a copy of main model error on current translations to be more realistic
We have changed validate_translation method on Globalize concern.
The objective is skip length validations when locale is distinct to
default_locale.
First we force apply :length validations when locale is equal to
default_locale. After we reject :length from options and apply rest
of validation options only when we have more than 1 options.
Ej: options = { length: "maximum: 10" }
When reject :length option in this example, options is equal to a
empty hash and we cant execute validations.
As we cannot order budget investments by any translatable field through
AR queries we are doing the same using ruby Array sort method and doing
array pagination manually with Kaminari 'paginate_array' helper method.
This method will be used by any translatable model that uses pg_search
feature so it's better to have it within globalizable model concern so
all translatable models can use it.
Paranoia is activated on translation classes by reflection, this is
making Rails to load translation classes before to execute migration
that adds the new column.
With this extra check Rails will not execute this code until translation
table has this column created.
Move method from sanitizable to globalizable concern because
globalize_accessors were overiding sanitizable method and was never
called. Another solution to this could be to load sanitizable
always after globalizable concern.
Old method implementation was not working well with globalize_accessors,
it was returning nil always.
Since Globalize gem update to v5.2.0 we cannot override translations
anymore in the same way that before the update. Milestone::Translation
class removed in this commit were no longer loaded correctly when
translation class is retrieved by translation_class method provided by
Globalize. Here is the diff between both gem versions:
https://github.com/globalize/globalize/compare/v5.0.0...v5.2.0#diff-a1370b109e0dd567545b072bc6447b8fR51
This problem is not happening on test environment but is throwing an
exception in other environments as it has not loaded the delegation
definition inside our custom translation class.
To fix this we added a new class method inside globalizable model
concern to allow to define method delegation on translations classes from
parent globalizable classes when needed without having to override
Translation classes.
Another way to properly load our custom Milestone::Translation class is
to place it inside parent model class, like the example below:
class Milestone
...
class Translation
delegate :status_id, to: :globalized_model
end
end
Or maybe monkey patching translation_class method from globalize gem
to make it find our custom translation class. I don't like this option.
This way we guarantee there will be at least one translation for a model
and we keep compatibility with the rest of the application, which
ideally isn't aware of globalize.
