Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to use custom presenters for download links #8255

Conversation

k0va1
Copy link

@k0va1 k0va1 commented Feb 6, 2024

Sometime we want to be able to change download_links configuration for different actions. For example:

index download_links: [:json] do
...
end

show do
...
end

Now download_links config from index works for both index and show actions. My PR allows to specify download_links separately

javierjulio and others added 13 commits October 9, 2023 16:22
* Remove link rake task

All except for one was to do with whitespace. We want to reduce code footprint and use off-the-shelf tools for basic checks like these. Since they aren't critical we are fine without replacements other than what eslint and rubocop already offer.

The sass-rails lint was for webpacker but we have already removed webpacker support. We won't be using any images or asset URLs in the new v4 approach for assets.

* Add metadata to activeadmin.gemspec (activeadmin#8012)

* Optimize data generation with insert_all

* Mention Ransack allowlist in documentation (activeadmin#8043)

* Update .gitignore

* Update all outdated dependencies

Ran `bin/bundle update`

Ran `yarn upgrade-interactive --latest`

Note this updates the default gemfile to use Rails 7.1 but we are good with that for our 3-0-stable branch.

---------

Co-authored-by: John DeSilva <desilvjo@umich.edu>
Co-authored-by: Francesco Belladonna <francesco@fc5.me>
Remove upper bound limits in gemspec

Other than the gems we own, remove the upper bound limits to ease maintenance. In some cases these can update (e.g. support new Rails version) without affecting ActiveAdmin, Arbre or InheritedResources so we want to allow that to reduce maintenance effort and to also allow users to try upgrading and submit fixes if needed.
…in#8105)

replace to_formatted_s with to_s to convert date to string

Solves the following deprecation warning in Rails 7.1:
```
DEPRECATION WARNING: to_default_s is deprecated and will be removed from Rails 7.2 (use to_s instead)
```

Check deprecation
[here](https://github.com/rails/rails/blob/v7.1.1/activesupport/lib/active_support/core_ext/date/conversions.rb#L59)

It shouldn't be a problem in [Rails 6.1](https://github.com/rails/rails/blob/v6.1.7.6/activesupport/lib/active_support/core_ext/date/conversions.rb#L59
) or [Rails 7.0](https://github.com/rails/rails/blob/v7.0.8/activesupport/lib/active_support/core_ext/date/conversions.rb#L59
)
* run ci against rails 7.1

* change test_app name

* make test app generation work with rails 7.1 and older

* resolve ransack 'NoMethodError: undefined method '

* formtastic 'DEPRECATION WARNING: Calling silence on ActiveSupport::Deprecation is deprecated and will be removed from Rails'

* do not check redirect body anymore because rails 7.1 removed it

see rails/rails@c2e756a

* temporal: resolve cucumber-rails DEPRECATION WARNING: Setting action_dispatch.show_exceptions to false is deprecated. Set to :none instead

* enable reloading to be able to test 'class-reload in development'

* bump devise from 4.9.2 to 4.9.3

* Fix tabs_spec flaky tests by allow lazy load of transliteration rules

Run the following code prior this commit to see the error

```
rspec ./spec/unit/views/components/tabs_spec.rb:42 --seed 57923
```

The error:

```
Failure/Error: result = string.parameterize

       I18n received :t with unexpected arguments
         expected: (:tab_key)
              got: (:"i18n.transliterate.rule", {:default=>{}, :locale=>:en, :resolve=>false})
       Diff:
       @@ -1 +1 @@
       -[:tab_key]
       +[:"i18n.transliterate.rule", {:default=>{}, :locale=>:en, :resolve=>false}]

     # /home/matias/.rvm/gems/ruby-3.2.2/gems/i18n-1.14.1/lib/i18n/backend/transliterator.rb:13:in `transliterate'
     # /home/matias/.rvm/gems/ruby-3.2.2/gems/i18n-1.14.1/lib/i18n.rb:298:in `transliterate'
     # /home/matias/.rvm/gems/ruby-3.2.2/gems/activesupport-7.1.0/lib/active_support/inflector/transliterate.rb:84:in `transliterate'
     # /home/matias/.rvm/gems/ruby-3.2.2/gems/activesupport-7.1.0/lib/active_support/inflector/transliterate.rb:125:in `parameterize'
     # /home/matias/.rvm/gems/ruby-3.2.2/gems/activesupport-7.1.0/lib/active_support/core_ext/string/inflections.rb:216:in `parameterize'
     # ./lib/active_admin/views/components/tabs.rb:35:in `fragmentize'
     # ./lib/active_admin/views/components/tabs.rb:19:in `build_menu_item'
     # ./lib/active_admin/views/components/tabs.rb:9:in `tab'
```

The problem?

tab calls `parameterize` on the give title. That use `I18n.transliterate` under the hood which calls `I18n.t`.
We must consider that call in our stub.

Why is a flaky tests?

Because `I18n` uses an internal cache. Only the first call to `I18n.transliterate` calls `I18n.t`.

See

https://github.com/rails/rails/blob/v7.1.1/activesupport/lib/active_support/core_ext/string/inflections.rb#L216
https://github.com/rails/rails/blob/v7.1.1/activesupport/lib/active_support/inflector/transliterate.rb#L123
https://github.com/rails/rails/blob/v7.1.1/activesupport/lib/active_support/inflector/transliterate.rb#L64
https://github.com/ruby-i18n/i18n/blob/v1.14.1/lib/i18n/backend/transliterator.rb#L13

* require ransack 4.1

* Revert "require ransack 4.1"

This reverts commit 51f665e.

This would force users to drop ruby 2.7
…error for filters (activeadmin#8117) (activeadmin#8163)

Give more info about db timeout error in filters

When a database timeout error occurs querying the values for
a filter, the error message does not give the filter information

Co-authored-by: Vicente Correa <vcorrea@buk.cl>
…torage is used (activeadmin#8137) (activeadmin#8164)

feat: check ransackable_associations when checking searchable_has_many_through

Co-authored-by: Edmund Tay <edmundtay96@gmail.com>
…lter (activeadmin#8089) (activeadmin#8165)

Add support for citext column types

Co-authored-by: Francesco Belladonna <francesco@fc5.me>
… menu options (activeadmin#8132) (activeadmin#8166)

Make sure menu creation does not modify menu options (activeadmin#8078)

This is an attempt to fix/work around activeadmin#8078 and related
issues.

Co-authored-by: dssjoblom <dssjoblom@gmail.com>
Co-authored-by: Daniel Sjöblom <daniel@ziney.co>
@mgrunberg
Copy link
Contributor

@k0va1 thanks for the PR. Would you add some test? Maybe here is a good place.

Also, would you make the PR against the default branch? We can backport the enhancement later

@k0va1 k0va1 changed the base branch from 3-0-stable to master February 8, 2024 09:39
@mgrunberg mgrunberg changed the base branch from master to 3-0-stable March 1, 2024 22:12
@mgrunberg mgrunberg changed the base branch from 3-0-stable to master March 1, 2024 22:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants