After the previous commit, we were using `double` in many places but
were only using `instance_double` in one file. So, for consistency,
we're using `double` in this file as well.
Lately (not sure since when), from time to time we've been getting these
failures in our CI:
```
Failures:
1) CommentsHelper#comment_author_class returns is-author if author is the commenting user
Failure/Error: comment = instance_double(Comment, user_id: author_id)
the Comment class does not implement the instance method: user_id
# ./spec/helpers/comments_helper_spec.rb:48:in `block (3 levels) in <top (required)>'
# ./spec/spec_helper.rb:40:in `block (3 levels) in <top (required)>'
# ./spec/spec_helper.rb:39:in `block (2 levels) in <top (required)>'
2) CommentsHelper#comment_author_class returns an empty string if commenter is not the author
Failure/Error: comment = instance_double(Comment, user_id: author_id - 1)
the Comment class does not implement the instance method: user_id
# ./spec/helpers/comments_helper_spec.rb:55:in `block (3 levels) in <top (required)>'
# ./spec/spec_helper.rb:40:in `block (3 levels) in <top (required)>'
# ./spec/spec_helper.rb:39:in `block (2 levels) in <top (required)>'
```
It might be related to the upgrade of rspec-rails done in commit
6fe222148 or maybe due to a change in github actions that caused some
tests to fail, as described in commits bedcb5bca and 3e44eeaee.
What might be causing the issue is the usage of `instance_double`
stubbing different methods in different tests (not sure this is the
cause, though).
We've seen that somebody got a similar error [1] (although it might not
have been for the same reason) and one of the maintainers of rspec-mocks
replied:
> I would recommend switching to double (as you mentioned) or
> refactoring to use something more defined.
So we're simply using `double`, which is what we usually use when
stubbing objects in the tests. Doing so is faster than further
investigating why the `instance_double` isn't reliable 100% of the time.
[1] See issue 1587 in https://github.com/rspec/rspec-mocks/
- Created `object_by_id_field` method in `BaseObject` to simplify the declaration of fields
with an `id` argument.
- Replaced all instances of `field ... do` blocks with `object_by_id_field` where fields require
an `id` argument across multiple types.
Note: Since we update to 1.80.1 deprecation warnings are appear when execute the assets:precompile command.
In order to silence this deprecation, we add silence_deprecation option in sass.rb initializer.
The code has also been updated to remove the deprecation warnings that appeared related to the function
darken(), lighten() and "Using / for division" instead of the function calc().
Bumps [sassc-embedded](https://github.com/sass-contrib/sassc-embedded-shim-ruby) from 1.70.1 to 1.80.1.
- [Commits](https://github.com/sass-contrib/sassc-embedded-shim-ruby/compare/v1.70.1...v1.80.1)
---
updated-dependencies:
- dependency-name: sassc-embedded
dependency-type: direct:production
update-type: version-update:semver-minor
...
Signed-off-by: dependabot[bot] <support@github.com>
When the `multitenancy_management_mode` is enabled.
In order to avoid infinite redirects when regular users try to access
the admin section, we're redirecting to the account page in this case.
Otherwise, the admin section would redirect to the root path, which
would redirect to the admin section, which would redirect to the root
path, and so on.
We only want to render the account link and login items in the header.
And we want only render the Multitenancy and Administrators sections in
the admin sidebar.
We include the administrators management so it's possible to give
permissions to other users to manage tenants.
In order to restrict access to other sections by typing the URL or
following a link, we're only enabling the rest of the routes when we
aren't in the multitenancy management mode.
We're going to add some constraints in the routes file, and if we add a
`resolve` clause inside a constraints block, we get an error saying that
"The resolve method can't be used inside a routes scope block" when
starting the application.
- Introduced `run_graphql_field` in tests that focus on resolving specific fields, leveraging the method added in GraphQL 2.2.0.
- Continued using `execute` for broader cases where it is still necessary to test entire GraphQL queries.
There are many possible ways to implement this feature:
* Adding a custom middleware
* Using rack-attack with a blocklist
* Using routes constraints
We're choosing to use a controller concern with a redirect because it's
what we do to handle unauthorized cancancan exceptions.
Note: The parser error message format changed in GraphQL 2.2.0 due to the introduction
of a new optimized lexer and a hand-written parser (PR 4718). This commit updates
the `parser_error_raised?` method in the GraphqlController tests to correctly detect
errors using the new message format.
The previous pattern was checking for "Parse error on", but with the new version,
the error message now contains "Expected one of". This change ensures that the
tests for malformed queries continue to pass as expected.
Bumps [graphql](https://github.com/rmosolgo/graphql-ruby) from 2.0.31 to 2.3.18.
- [Release notes](https://github.com/rmosolgo/graphql-ruby/releases)
- [Changelog](https://github.com/rmosolgo/graphql-ruby/blob/master/CHANGELOG.md)
- [Commits](https://github.com/rmosolgo/graphql-ruby/compare/v2.0.31...v2.3.18)
---
updated-dependencies:
- dependency-name: graphql
dependency-type: direct:production
update-type: version-update:semver-major
...
Signed-off-by: dependabot[bot] <support@github.com>
Using a checkbox wasn't very intuitive because checkboxes are
checked/unchecked when clicked on even if there's an error in the
request. Usually, when checkboxes appear on a form, they don't send any
information to the server unless we click a button to send the form.
So we're using a switch instead of a checkbox, like we did to
enable/disable phases in commit 46d8bc4f0.
Note that, since we've got two switches that match the default
`dom_id(record) .toggle-switch` selector, we need to find a way to
differentiate them. We're adding the `form_class` option for that.
Also note that we're now using a separate action and removing the
JavaScript in the `update` action which assumed that AJAX requests to
this action were always related to updating the `visible_to_valuators`
attribute.
We were performing 3 requests in order to refresh the page and check the
content was still there. We can use `refresh` instead.
We're also reusing the `investment1` variable in every test, instead of
redifining it in one of them.
This is consistent to what we usually do. Also, we're applying the same
criteria mentioned in commit 72704d776:
> We're also making these actions idempotent, so sending many requests
> to the same action will get the same result, which wasn't the case
> with the `toggle` action. Although it's a low probability case, the
> `toggle` action could result in [selecting an investment] when trying
> to [deselect] it if someone else has [deselected it] it between the
> time the page loaded and the time the admin clicked on the
> "[Selected]" button.
Just like it happened with proposals, the button to select/deselect an
investment wasn't very intuitive; for example, it wasn't obvious that
pressing a button saying "selected" would deselect the investment.
So we're using a switch control, like we do to enable/disable features
since commit fabe97e50.
Note that we're making the text of the switch smaller than in other
places because the text in the investments table it is also smaller
(we're using `font-size: inherit` for that purpose). That made the
button look weird because we were using rems instead of ems for the
width of the button, so we're adjusting that as well.
Also note we're changing the width of the switch to `6em` instead of
`6.25em` (which would be 100px if 1em is 16px). We're doing so because
we used 100 for the minimum width because it's a round number, so
now we're using another round number.
Since we were checking something that was already there, if the form
were submitted using an AJAX request (which is the default with Turbo,
although we don't use it yet), there would be a chance that the request
didn't finish before the test does, leading to potential failures in the
next test.
This way we're also checking that the "Filter" button actually does
something when selecting the "Selected" filter.
We don't need to replace the whole row, since the changes only affect
the button. Therefore, we don't need to depend on an `inserted` event to
decide which columns to render in that row.
We were checking it in the view, meaning that it was possible to toggle
the selection by sending a custom request even when the investment
wasn't feasible.
This way it'll be easier to change the link/button used to toggle the
selection.
Note that the conditions in the view seem to be different because we no
longer include the `selected?` condition when rendering the link/button.
However, an investment can only be selected if it's feasible and its
valuation is finished, so writing something like this would have been
redundant:
```ruby
can?(:toggle_selection, investment) &&
(selected? || investment.feasible? && investment.valuation_finished?)
```
The reason why the previous code was using the `selected?` condition was
to check whether to render the link/button to select or to deselect an
investment. We're now doing that in the Ruby part of the component.
