There are two reasons for this change:
1. Past migrations depending on models will not work once a model is
removed, and they won't work if we remove Globalize either
2. We were getting a conflict in the schema file; when run under Rails
5.0, these migrations were generating a different schema than in Rails
5.1, due to the way the `create_translation_table!` method handles the
`id: :serial` attribute.
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.
CKEditor repositions the dialog to attach an image after showing it. If
its position changes right when Capybara is trying to click the "Upload"
link, the click does not work properly.
I haven't found a way to check the dialog has stopped moving, so I've
chosen to force its position to the top.
Without this change, this test failed a lot on my machine when using
Rails 5.1, both with an "iframe ckeditor" and an "inline ckeditor".
However, it didn't fail when using Rails 5.0.
Rails 5.1 updated the `method_source` dependency, which is incompatible
with pry 0.12.x (which some developers are using), and upgrading pry and
pry-byebug requires a more recent version of byebug.
Since we use transactions now for every test, we can seed the database
at the beginning, and then it will go back to this state before a test
is executed.
Running the test suite is now considerably faster. On my machine, we
save a quarter of second per system test, meaning we save several
minutes for the whole suite.
We were manually setting `http://www.example.com`. However, Capybara now
uses `http://127.0.0.1`.
While we could change the code to use `127.0.0.1`, I think directly
using `Capybara.app_host` makes it easier to realize what the code is
doing. And, particularly, now it's clear the host has nothing to do with
our `Setting["url"]`, which by default points to `www.example.com` as
well.
This method is deprecated in Rails 5.1 because its behavior will be
different in `before` and `after` callbacks.
We're replacing the deprecated `attribute_changed?` and `attribute_was`
methods with `saved_change_to_attribute?` and
`attribute_before_last_save` during `after_save` callbacks.
https://github.com/rails/rails/pull/32835/
This method is deprecated in Rails 5.1 because its behavior will be
different in `before` and `after` callbacks.
Here we're replacing the deprecated `attribute_changed?` and
`attribute_was` with `will_save_change_to_attribute?` and
`attribute_in_database` during `before_save` callbacks.
https://github.com/rails/rails/pull/32835/
Rails 5.1 introduced certain changes in the way a record is touched when
the counter cache option is enabled in a belongs to association.
We need to upgrade acts-as-taggable-on so it keeps changing the
`updated_at` attribute when a new tag is added to a record.
Note we now need to reload the records in some cases to get the
`context_tag_list` method to return what we expect. Methods like
`context_tags` however work properly with no need to reload the record.
Rails 5.1 doesn't align the columns in the schema file anymore, so
there aren't unrelated changes when we add or remove columns to a table:
https://github.com/rails/rails/pull/25675
We're sorry for developers who are really concerned about code
alignment.
Note we need to upgrade the bullet gem, although another option would be
to remove it completely.
Now we don't need the rubocop rules for deprecated methods, since using
them will raise an error and we'll be notified immediately.
This plugin provides more control over tables and solves a JS error thrown
when user clicks on "Cell properties" ckeditor feature.
https://ckeditor.com/cke4/addon/tabletools
All of these plugins are not used anywhere.
Change introduced at ckeditor initializer will ommit unneeded
precompilation of plugins assets on production environments.
Change introduced at ckeditor config file adresses the problem with assets
pipeline fallback on testing environments described here: #2711. Now plugins
that are explicitly disabled will not be precomiled when running ckeditor
javascript enabled feature specs.
The URL used for the generated request was
`/ckeditor/pictures&responseType=json`. This is a known issue in the
ckeditor gem, and it's suggested to add a `?` at the end of the URL in
order to fix it.
I haven't added a test for this case since simulating dropping a file in
the browser with Selenium/Capybara seems to be quite tricky and I
haven't found a solution guaranteed to correctly emulate what users do.
This feature was not working so its better to disable it completely.
By removing attachment_files endpoints related buttons and tabs from ckeditor UI are not shown to users anymore.
We were getting a warning: "Checking for expected text of nil is
confusing and/or pointless since it will always match. Please specify a
string or regexp instead" because we were checking for values which were
set to `nil` in the tests.
Since we're only doing the convertion from bytes to megabytes in one
place, IMHO adding an extra method makes the code harder to read.
This way we don't have do include the DocumentsHelper in the specs
anymore, reducing the risk of possible method naming collisions.
We were converting megabytes to bytes with the `megabytes` method and
then adding a `bytes_to_megabytes` method to convert it back to bytes,
which is the same as not doing anything at all :).
Including them might lead to conflicts since two methods might have the
same name. For example, we're getting some exceptions when taking
screenshots of a failing test, because the method `image_path` from
`ActionView::Helpers::AssetUrlHelper` has the same name as a method used
to save the screenshot.
Besides, we were including all helpers in places were only the `dom_id`
method is used, and in other places where no helper methods were used at
all. So we can just invoke `ActionView::RecordIdentifier.dom_id`
directly.
Implementation tries to be open for further extensions, such as deciding on
search dictionary based on configuration option or by locale set for
given user.
The method `tag_list_on` doesn't add an `ORDER_BY` clause to the SQL
query it generates, and so results may come in any order.
However, in the tests we were assuming the tags were ordered by ID in
descending order. Since that isn't always the case, the tests were
failing sometimes.
Ordering the tags alphabetically solves the problem. We could also use
the same order admins used when adding the tags:
```
@process.customs.order("taggings.created_at").pluck(:name).join(", ")
```
However, I'm not sure it improves the user experience, and it makes the
code more complicated.
benefit to administratos.
It looks like sometimes, particularly when the first thing we do after
loading a page is filling the CKEditor fields and submitting the form,
CKEditor doesn't have enough time to format the text, and so it's sent
as plain text instead of HTML. This behaviour can be reproduced on my
local machine after upgrading to Rails 5.1, with the test "Admin Active
polls Add" failing 100% of the time.
Checking CKEditor has been filled in correctly solves the issue.
When an investment had been assigned a user tag and a valuation tag with
the same name, it appeared twice when filtering by tag.
This is because by design, in order to provide compatibility with scopes
using "select" or "distinct", the method `tagged_with` doesn't select
unique records.
Forcing the query to return unique records solves the issue.