Commit Graph

18 Commits

Author SHA1 Message Date
taitus
d499a6944e Fix typos in success messages 2022-09-20 17:29:04 +02:00
taitus
86ff4eeff9 Add consistency to back_links
In some cases (e.g. after editing or creating a resource
with errors) the default back_link did not redirect to the
expected page.

Now we force the back_links to the index pages, so we
always get the desired redirect.
2022-09-14 14:56:27 +02:00
taitus
b17b03099c Load videos through answer in all actions 2022-09-14 14:45:36 +02:00
taitus
1ec793863b Add missing system specs for poll question answer videos
Minor change for replace @video.answer_id to @video.answer.
2022-09-14 14:45:35 +02:00
Julian Herrero
24276a201e Remove useless answers show view
Until now, in order to edit an answer, we had to click on its title on
the table and then on the "Edit answer" link.

That was tedious and different from what we usually do in the admin
section. Furthermore, the code for the answers table was written twice
and when we modified it we forgot to update the one in the `show`
action, meaning the table here provided less information than the
information present in the answers tables.

Co-Authored-By: Javi Martín <javim@elretirao.net>
2022-09-14 14:45:35 +02:00
taitus
ec861ca8e6 Extract documents action from Answer controller
This way we have a controller just to manage
Poll::Question::Answer related documents in the
same way we have for videos and images.
2022-09-14 14:45:34 +02:00
taitus
405b37f605 Load answer through question in answers controller
We are simplifying the load answer and we can remove the ambiguous
hidden field from answer form.
2022-09-14 14:45:33 +02:00
Javi Martín
4f232c3a25 Use the file_fixture helper in tests
This way we don't have to write `"spec/fixtures/files"` every time.

Note this method isn't included in factories. We could include it like
so:

```
FactoryBot::SyntaxRunner.class_eval do
  include ActiveSupport::Testing::FileFixtures
  self.file_fixture_path = RSpec.configuration.file_fixture_path
end
```

However, I'm not sure about the possible side effects, and since we only
use attachments in a few factories, there isn't much gain in applying
the monkey-patch.
2022-02-23 18:43:48 +01:00
rhian-cs
609e58cacb Update system specs with detailed confirmation alerts 2021-12-22 12:32:47 +01:00
rhian-cs
b27a05cbfd Make confirmation alert message inform the triggering action 2021-12-22 12:32:45 +01:00
Javi Martín
6f219beff0 Remove unused parameter in imageable tests method 2021-09-24 16:36:35 +02:00
Javi Martín
5311daadfe Use a button for non-GET table actions
Links acting like buttons have a few disadvantages.

First, screen readers will announce them as "links". Screen reader users
usually associate links with "things that get you somewhere" and buttons
with "things that perform an action". So when something like "Delete,
link" is announced, they'll probably think this is a link which will
take them to another page where they can delete a record.

Furthermore, the URL of the link for the "destroy" action might be the
same as the URL for the "show" action (only one is accessed with a
DELETE request and the other one with a GET request). That means screen
readers could announce the link like "Delete, visited link", which is
very confusing.

They also won't work when opening links in a new tab, since opening
links in a new tab always results in a GET request to the URL the link
points to.

Finally, submit buttons work without JavaScript enabled, so they'll work
even if the JavaScript in the page hasn't loaded (for whatever reason).

For all these reasons (and probably many more), using a button to send
forms is IMHO superior to using links.

There's one disadvantage, though. Using `button_to` we create a <form>
tag, which means we'll generate invalid HTML if the table is inside
another form. If we run into this issue, we need to use `button_tag`
with a `form` attribute and then generate a form somewhere else inside
the HTML (with `content_for`).

Note we're using `button_to` with a block so it generates a <button>
tag. Using it in a different way the text would result in an <input />
tag, and input elements can't have pseudocontent added via CSS.

The following code could be a starting point to use the `button_tag`
with a `form` attribute. One advantage of this approach is screen
readers wouldn't announce "leaving form" while navigating through these
buttons. However, it doesn't work in Internet Explorer.

```
ERB:

<% content_for(:hidden_content, form_tag(path, form_options) {}) %>
<%= button_tag text, button_options %>

Ruby:

def form_id
  path.gsub("/", "_")
end

def form_options
  { id: form_id, method: options[:method] }
end

def button_options
  html_options.except(:method).merge(form: form_id)
end

Layout:

<%= content_for :hidden_content %> # Right before the `</body>`
```
2021-09-20 20:27:37 +02:00
Javi Martín
41a9d17c76 Add and apply Lint/SymbolConversion rubocop rule
This rule was added in Rubocop 1.9.0.

We're excluding the Setting model in order to keep the settings
consistent.
2021-09-03 11:49:53 +02:00
Javi Martín
92ddcb7aef Use JavaScript in system tests by default
JavaScript is used by about 98% of web users, so by testing without it
enabled, we're only testing that the application works for a very
reduced number of users.

We proceeded this way in the past because CONSUL started using Rails 4.2
and truncating the database between JavaScript tests with database
cleaner, which made these tests terribly slow.

When we upgraded to Rails 5.1 and introduced system tests, we started
using database transactions in JavaScript tests, making these tests much
faster. So now we can use JavaScript tests everywhere without critically
slowing down our test suite.
2021-04-07 14:41:06 +02:00
Javi Martín
287c488734 Use JavaScripts in tests using CKEditor
We were filling in textareas, so we were only testing how the
application behaves for about 1%-2% of our users.
2021-04-07 14:41:06 +02:00
Javi Martín
3da4ee00b8 Simplify tests requiring admin login
We were repeating the same code over and over (with a few variants) to
setup tests which require an administrator. We can use a tag and
simplify the code.
2020-12-02 15:33:19 +01:00
Javi Martín
37361a6f3d Replace render :nothing with head :ok
Using `render :nothing` was deprecated, but we never noticed it because
we didn't have a test for the action using it. In Rails 5.1, it raises
an exception.

Using `head :ok` and adding a test for this scenario solves the issue.
2020-04-24 15:43:54 +02:00
Javi Martín
9427f01442 Use system specs instead of feature specs
We get rid of database cleaner, and JavaScript tests are faster because
between tests we now rollback transactions instead of truncating the
database.
2020-04-24 15:43:54 +02:00