The new CSV report was more configurable and could work on proposals,
processes and comments. However, it had several issues.
In the public area, by default it generated a blank file.
In the admin section, the report was hard to configure and it generated
a file with less quality than the old system.
So until we improve this system, we're bringing back the old investment
CSV exporter.
This commit reverts most of commit 9d1ca3bf.
The current tracking section had a few issues:
* When browsing as an admin, this section becomes useless since no
investments are shown
* Browsing investments in the admin section, you're suddenly redirected
to the tracking section, making navigation confusing
* One test related to the officing dashboard failed due to these changes
and had been commented
* Several views and controller methods were copied from other sections,
leading to duplication and making the code harder to maintain
* Tracking routes were defined for proposals and legislation processes,
but in the tracking section only investments were shown
* Probably many more things, since these issues were detected after only
an hour reviewing and testing the code
So we're removing this untested section before releasing version 1.1. We
might add it back afterwards.
We need to add a hidden field for each group of check boxes, so if we
don't check anything, the hidden field is sent to the server, indicating
nothing was selected. Without the hidden field, the server will not know
anything has been done to the check boxes.
The easiest way to do it is using `collection_check_boxes`, which also
adds labels to every check box.
We were using two different systems to set translations in JavaScript:
to set the text for languages, we were using data attributes, and to set
the text for staff members, we were using AJAX calls.
I find data attributes keep the code more simple, since there's no need
to define an extra route and controller action. Furthermore, the user
experience is better because response times are faster.
So now both places use data attributes.
Unfortunately this feature wasn't properly reviewed and tested, and it
had many bugs, some of them critical and hard to fix, like validations
being skipped in concurrent requests.
So we're removing it before releasing version 1.1. We might add it back
in the future if we manage to solve the critical issues.
This commit reverts commit 836f9ba7.
We're going to change CKEditor to an inline editor, and the "ckeditor"
gem doesn't provide an option to do so.
Since using `cktext_area` would automatically generate a "classic"
iframe CKEditor, we need to use `text_area` and load the editor using
JavaScript. Personally I prefer this option anyway.
Note in the jQuery selector we need to use `textarea.html-area`; using
just `.html-area` would fail if there's an error message associated to
the textarea, since Rails will add the `.html-area` class to the error
message.
The jQuery html() function does not filter <script> tags, so if somehow
an attacker introduced a <script> in the translation, we would be
vulnerable to a XSS attack.
Note using $.parseHTML wouldn't solve the problem, since it doesn't
filter attributes in image tags.
Since changing the text of the part which doesn't have the count wasn't
very clean, I've added another <span> tag for the part with the
description, and so we can use jQuery's text() function to replace it.
Using html() makes it possible to insert <script> tags in the DOM, and
in this case we aren't supposed to be inserting any HTML.
I haven't found a way to focus on a field with Capybara, then add a
character, and focus on another field. So I've manually triggered the
change event in the test.
Moderate legislation proposals
- added a controller for moderation/legislation
- updated view to appropriate link + added route
- added a spec
- Feature test
- test for faded
- javascripts for visual effects
When we were inserting a row or replacing an existing one (just like we
do when we click the link to select an investment), we were entering a
row containing all columns, and all of them were displayed even if they
had been excluded using the column selector.
This caused the table to move in a strange way, which sometimes made the
investment selection tests fail.
Using the same variable name makes the code more difficult to read.
We're also enabling the `no-param-reassing` rule since we accidentally
reassigned params in commit 824dd26d, and this rule will prevent us from
doing so in the future.
We originally wrote `undefined` in CoffeeScript.
CoffeeScript compiled it to `void 0` when generating the JavaScript
files. However, the reason to do so was the `undefined` variable was
mutable before ECMAScript 5. Using `undefined` is more intuitive, and
nowadays there's no reason to use `void 0`.
For now we're only adding rules related to spacing and double quotes,
following the same rules we use in Ruby, which are the same rules
CoffeeScript followed when compiling these files.
We're also using the recommended ESLint rules, which will warn us about
many JavaScript common pitfalls, the `strict` rule which enforces using
strict mode, and the `no-console` rule, which will prevent us from
shipping code meant for debugging.
Although it's arguably more common to use the JSON format to define
these rules, I've chosen YAML because it's the format we use in all our
linters.
These statements were automatically added by CoffeeScript.
I'm only removing the obvious cases; there might be more cases where the
`return` statement isn't necessary.
Local variables are one of the things CoffeeScript doesn't compile to
modern JavaScript automatically: it uses `var` instead of `const` or
`let`.
Besides, using `$this = $(this)` is usually done to reference the
current object in another function where the current object is a
different one. Here we were using it with no clear purpose.
In JavaScript we cannot easily repeat something N times, and
CoffeeScript's range loop compiles into complex JavaScript.
We could also use `Array.from({ length: N })`, but that's not supported
by old browsers like Internet Explorer 11.
Iterating through the jQuery elements with `each` is more similar to
other iterators we use. Furthermore, we avoid declaring the `index`
variable we don't use.
The `append` method can handle arrays, so there's no need to loop
through each element.
There's more to improve on this method, like the `asc` variable being
global to a table instead of depending on the current column, but for
now I'm only refactoring and maintaining the current behaviour.
These methods were added in 2009 and are supported by 98.5% of the
browsers, including Internet Explorer 9, 10 and 11. We were already
using them in some places.
Calling it directly might make it difficult to detect bugs, particularly
when we don't trust the data we receive:
https://eslint.org/docs/rules/no-prototype-builtins
If we trust the data we receive, then IMHO we shouldn't even check for
`hasOwnProperty`, since we know what we're going to receive.
In JavaScript, when there isn't a `break` or `return` statement inside a
`switch` case, the next case will be executed as well.
That wasn't a problem here because CoffeeScript automatically inserts a
`return` statement in this specific situation. However, since we don't
want to return the result of the `hide()` operation, it might be easy to
accidentally remove the `return` statement, causing the code to break.
I've added a test for the scenario where neither `break` nor `return`
statements are present, so we don't run into this error.
This code requires the variable `_paq` to be set somewhere, but we never
set it.
In the past Decide Madrid added some custom JavaScript using this code.
However, in CONSUL we're using Ahoy to track events, and we don't have
any documentation about adding custom JavaScript to use piwik nor we've
got any other piwik integration.
The `removeCookie` function is never called, and the `initialize`
function doesn't do anything. The only functions we use here are
`getCookie` and `saveCookie`.
