We removed the link to this page in commit 83e8d6035 because poll
questions don't really make sense without a poll.
However, this page also contained information about successful
proposals, which might be interesting so administrators don't have to
navigate to the public area in order to find and create questions based
on successful proposals.
So we're keeping the part about successful proposals and linking it from
the proposals part of the admin area.
Note we're using translation keys like `successful_proposals_tab`, which
don't make sense anymore, for the successful proposals. We're doing so
because we've already got translations for these keys and, if we renamed
them, we'd lose the existing translations and our translators would have
to add them again.
Also note we're changing one poll question test a little bit so we
create the question from a successful proposal using the new page. There
are other tests checking how to create a question from the
admin/proposals#show action and other tests checking what happens when
accessing a successful proposal in the admin section, so we don't lose
any test coverage by changing an existing test instead of adding a new
one.
Finally, note that we've removing the `search` method in poll question
because we no longer use it. This currently makes the
`author_visible_name` database column useless; we aren't removing it
right now because we don't want to risk a possible data loss in a patch
release (we're about to release version 2.3.1), but we might remove it
in the future.
I accidentally made this mistake while trying to avoid the exact same
issue back in commit 73992b2c8. Accessing the database in a test after
starting the process running the browser has caused database corruption
in our CI multiple times.
This parameter isn't used since commit b4a6f664b.
Note we're changing the tests to use proposals instead of debates
because proposals may have images attached, while debates may not.
This way we know for sure the page has finished refreshing.
Note that, for now, we aren't adding a check after every call to the
`refresh` method because sometimes the page remains identical after
refreshing. Not sure what we should do in these cases.
After a `visit`, we were checking for content or filling in fields that
were already there before the `visit`, so we weren't 100% sure that the
request had finished before the test continued.
In the case of the verification tests, we were clicking the submit
buttons over and over without and then checking or interacting with
elements that were already there. Even though the button was disabled
between requests, meaning there wouldn't be simultaneous requests, it
was possible to interact with a form field before waiting for the
request to finish.
Some of these tests have recently failed on our CI, and it might be
because of that:
```
1) Admin budgets Edit Changing name for current locale will update the
slug if budget is in draft phase
Failure/Error: raise ex, cause: cause
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
1) Budgets creation wizard Creation of a multiple-headings budget by
steps
Failure/Error: expect(page).to have_content "Heading created
successfully!"
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
1) Custom information texts Show custom texts instead of default ones
Failure/Error: raise ex, cause: cause
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
1) Users Regular authentication Sign in Avoid username-email collisions
Failure/Error: raise ex, cause: cause
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
2) Verify Letter Code verification 6 tries allowed
Failure/Error: raise ex, cause: cause
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
2) Valuation budget investments Valuate Finish valuation
Failure/Error: raise ex, cause: cause
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
1) Users Delete a level 2 user account from document verification page
Failure/Error: raise ex, cause: cause
Selenium::WebDriver::Error::UnknownError:
unknown error: unhandled inspector error: {"code":-32000,
"message":"Node with given id does not belong to the document"}
(Session info: chrome=134.0.6998.35)
```
We're already testing the navigation in one test, and in some of these
tests we were checking, for instance, that the title of an investment is
present after a click, but since it is also present before that click,
that could lead to the test finishing before the request does.
This way it's more obvious what's going on.
Note that, in this case, the expectations were **not** true before
visiting the page, so we aren't fixing a flaky test.
This way the code is easier to follow, and we avoid having simultaneous
requests on different browser sessions, which in theory shouldn't affect
the application, but the test might be a bit more robust this way.
This way the tests are slightly faster, and we avoid a possible flaky
spec since in one test we were checking that a certain text was not
present, which didn't guarantee that the request to delete the document
had finished.
After clicking the "Save changes" button, we were sometimes checking
expectations that were already true before clicking the button, so it
was possible that the request generated by that button didn't finish
before the test did.
So now we're checking that the button is no longer present, which is
only true after the request has finished.
The expectation that there isn't a link with the text "Español" wasn't
updated in commit f7417d647, when the tests were updated after switching
the user interface from a list of links to a select field.
We're also adding an additional expectation after the `visit` to make
sure the request has finished before selecting English from the selector
(which probably doesn't make sense because it's the only option, but
changing it is out of the scope of this commit).
We were checking that, after a `visit`, we're redirected to the account
page, but, since we were already in the account page, the page doesn't
change at all. In order to be 100% that the request caused by the call
to `visit` has finished before checking the expectations, we're visiting
a different page first.
We weren't checking whether the request creating the proposal had
finished before checking the document. That's probably why this test has
recently failed on our CI:
```
1) Documents Metadata download document without metadata
Failure/Error: io = URI.parse(
"#{app_host}#{polymorphic_path(
Document.last.attachment)}"
).open
NoMethodError:
undefined method `attachment' for nil:NilClass
```
We weren't checking that the requests had finished before checking the
last sent email. That's probably why one of these tests has recently
failed on our CI:
```
1) System Emails Preview Pending #send_pending
Failure/Error: email = open_last_email
RuntimeError:
No email has been sent!
```
These tests don't use the browser to send emails since commit e21588ec1.
However, note that this commit actually actually decreased our test
coverage somehow; since then, we're no longer testing whether we send an
email to the author after clicking the "Publish comment" button. We
might need to add a test for this in the `spec/system/comments_spec.rb`
file... but that's a story for another time.
Note we need to stub the `deliver_later` method; otherwise,
`open_last_email` would raise an exception since no email would have
been delivered.
We're moving them now because these tests use the `open_last_email`
method, and we're looking for places where using this method might
result in a flaky test when used inside a system test.
These tests were testing mailer methods, but were inside a file for
system tests.
We're moving them now because these tests use the `open_last_email`
method, and we're looking for places where using this method might
result in a flaky test when used inside a system test.
These tests were testing model methods, but were inside files for system
tests.
We're moving them now because these tests use the `open_last_email`
method, and we're looking for places where using this method might
result in a flaky test when used inside a system test.
This way it's more clear what the user experience is during the process.
Note this did not result in flaky tests, since the tests continue by
clicking links, that are only present after the request has finished.
However, we're adding expectations so we don't have to think whether the
tests could be flaky and because this way we're also testing the user
experience; it would be strange for a user if they were redirected to a
page without a flash message.
Quoting from commit 57bda006b5:
> When clicking the button "Search", the link "newest" is already
> present, so capybara might click the "newest" link before the "Search"
> request is finished, leading to unexpected results.
>
> Checking the page to make sure the "Search" request has finished
> before clicking the "newest" link solves the problem.
We're doing the same thing for the other tests that click the "Search"
button and then clicked on a link. We're also making sure the language
selector changes before clicking it again.
In most tests calling this method, we were doing another visit right
after calling this method, so by removing this `visit` call we're making
the tests slightly faster and easier to follow.
We weren't checking that the request caused by clicking on the "Send
instructions" button had finished before continuing with the test.
Perhaps that's why this test has recently failed on our CI:
```
3) Emails Reset password
Failure/Error: email = open_last_email
RuntimeError:
No email has been sent!
```
We're also adding an expectation to the `login_as_manager` method and
the methods to submit proposal and investment forms to make sure that,
when these method finish, the request finishes as well.
This way we're 100% sure we won't have simultaneous requests since the
test will wait for one request to finish before continuing.
In the debates spec, we're also making sure the expectations between the
two consecutive visits are different.
There were many cases where we were clicking on a link or (most of the
time) a button and then calling the `visit` method. In the past, it
worked just fine because clicking on buttons usually results in non-AJAX
requests, meaning that the test waited for the request to finish before
continuing.
That's no longer the case, though. In the last few months/years (not
sure since when) we're getting sporadic failures because the test
doesn't wait for the request to finish before making another request
with the `visit` method. This sometimes results in flaky tests.
Some of these tests have recently failed in our CI. Here are a few
examples (note the numbers don't follow an order because these tests
failed in different jobs):
```
1) Admin edit translatable records Current locale translation does not
exist For ActivePoll Shows first available fallback
Failure/Error: expect(page).to have_content "Sondage en Français"
expected to find text "Sondage en Français" in "Language: \n
\nEnglish\nDeutsch\nEspañol\nFrançais\nNederlands\nPortuguês
brasileiro\n中文\n Go back to CONSUL DEMOCRACY\nCONSUL
DEMOCRACY\nADMINISTRATION\nMenu\nNotifications\nMy content\nMy
account\nSign out\nProposals\nDebates\nComments\nPolls\n
Collaborative Legislation\nParticipatory budgets\nVoting booths
\nSignature Sheets\nMessages to users\nSite content\nModerated
content\nProfiles\nStatistics\nSettings\nProposals dashboard\n×
\nPolls description updated successfully.\nList of polls\nPolls
description\nCreate poll\nThere are no polls."
2) Public area translatable records Existing records Update a
translation With valid data Changes the existing translation
Failure/Error: expect(page).to have_field "Debate title",
with: "Title in English"
expected to find field "Debate title" that is not disabled but
there were no matches
2) Admin collaborative legislation Update Edit milestones summary
Failure/Error: expect(page).to have_content "There is still a long
journey ahead of us"
expected to find text "There is still a long journey ahead of us"
in "Language: \n
\nEnglish\nDeutsch\nEspañol\nFrançais\nNederlands\nPortuguês
brasileiro\n中文\n Go back to CONSUL DEMOCRACY\nCONSUL
DEMOCRACY\nADMINISTRATION\nMenu\nNotifications\nMy content\nMy
account\nSign out\nProposals\nDebates\nComments\nPolls\n
Collaborative Legislation\nParticipatory budgets\nVoting booths
\nSignature Sheets\nMessages to users\nSite content\nModerated
content\nProfiles\nStatistics\nSettings\nProposals dashboard\n×
\nProcess updated successfully. Click to visit\nBack\nAn example
legislation process\nInformation\nHomepage\nDebate\nProposals\n
Drafting\nFollowing\n1 language in use\nCurrent language\n
English\nSummary\n Format\n ◢\n Milestone\nManage progress
bars\nDon't have defined milestones\nCreate new milestone".
(However, it was found 1 time including non-visible text.)
3) Admin collaborative legislation SDG related list create Collaborative
Legislation with sdg related list
Failure/Error:
within("tr", text: "Legislation process with SDG related content") do
expect(page).to have_css "td", exact_text: "17"
end
Capybara::ElementNotFound:
Unable to find css "tr"
4) Valuation budget investments Valuate Feasibility can be marked as
pending
Failure/Error: expect(find("#budget_investment_feasibility_undecided"))
.not_to be_checked
Capybara::ElementNotFound:
Unable to find css "#budget_investment_feasibility_undecided"
3) Custom information texts Show custom texts instead of default ones
Failure/Error:
within("#section_help") do
expect(page).to have_content "Custom help with debates"
expect(page).not_to have_content "Help with debates"
end
4) Admin budgets Update Deselect all selected staff
Failure/Error: expect(page).to have_link "Select administrators"
expected to find link "Select administrators" but there were no
matches
3) Admin polls SDG related list edit poll with sdg related list
Failure/Error:
within("tr", text: "Upcoming poll with SDG related content") do
expect(page).to have_css "td", exact_text: "17"
end
Capybara::ElementNotFound:
Unable to find css "tr"
4) Admin polls SDG related list create poll with sdg related list
Failure/Error:
within("tr", text: "Upcoming poll with SDG related content") do
expect(page).to have_css "td", exact_text: "17"
end
Capybara::ElementNotFound:
Unable to find css "tr"
5) Admin custom images Image is replaced on admin newsletters
Failure/Error:
within(".newsletter-body-content") do
expect(page).to have_css("img[src*='logo_email_custom.png']")
end
Capybara::ElementNotFound:
Unable to find css ".newsletter-body-content"
6) Admin custom images Image is replaced on front views
Failure/Error:
within("#map") do
expect(page).to
have_css("img[src*='custom_map.jpg'][alt='Districts list']")
end
Capybara::ElementNotFound:
Unable to find css "#map"
```
We had inconsistent indentation in many places. Now we're fixing them
and adding a linter to our CI so we don't accidentally introduce
inconsistent indentations again.
We had some duplication because the `css_for_process_header` was using
an instance variable, and so it couldn't be called from a partial where
this instance variable wasn't available.
Using a local variable and passing it as a parameter (as we should
always do) solves the issue and lets us simplify the code.
Set cookie duration to 365 days based on the AEPD's cookie usage guidelines.
Note from the document: "Cookies with a duration of up to 24 months are
considered acceptable as long as they are periodically updated."
Reference: https://www.aepd.es/guias/guia-cookies.pdf
Create cookie consent "all" when accept all cookies
Set cookie duration to 365 days based on the AEPD's cookie usage guidelines.
Note from the document: "Cookies with a duration of up to 24 months are
considered acceptable as long as they are periodically updated."
Reference: https://www.aepd.es/guias/guia-cookies.pdf
Note that in order to avoid display duplicated vertical scroll when
render a modal, we are add an `overflow: unset` rule. This rule
overwrite a vendor rule both in the modal we are adding and in the
modal we already have when creating a budget in admin section.
Set cookie duration to 365 days based on the AEPD's cookie usage guidelines.
Note from the document: "Cookies with a duration of up to 24 months are
considered acceptable as long as they are periodically updated."
Reference: https://www.aepd.es/guias/guia-cookies.pdf
Allow enabling from settings admin section.
Note that we set the z-index to 20 in order to will be greater than
the others z-index elements in the application like <header> on
mobile devices.
Note that, while we're no longer including them as part of the
`execute_release_2.2.0_tasks` task, we're keeping the tasks to remove
duplicate poll voters and poll options just in case there are some
unexpected issues when adding a unique database index while upgrading to
version 2.3.0. We'll remove them in version 2.4.0.
According to the GeoJSON specification [1]:
> * A linear ring is a closed LineString with four or more positions.
> * The first and last positions are equivalent, and they MUST contain
> identical values; their representation SHOULD also be identical.
> (...)
> * For type "Polygon", the "coordinates" member MUST be an array of
> linear ring coordinate arrays.
Note that, for simplicity, right now we aren't checking whether the
coordinates are defined counterclockwise for exterior rings and
clockwise for interior rings, which is what the specification expects.
[1] https://datatracker.ietf.org/doc/html/rfc7946#section-3.1.6
Note we're adding a `name` property to the geozones investments sidebar
map even if we don't render the geozones in the map, in order to
simplify the JavaScript function `geozoneLayers`.
