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

Prevent creation of unnecessary fieldset(mirror and revised) #2449

Conversation

liijunwei
Copy link

@liijunwei liijunwei commented Aug 12, 2023

Purpose

Changes

  1. fieldset is not listed in lib/active_model_serializers/serializable_resource.rb@ADAPTER_OPTION_KEYS, remove the ||= protection (simplification)
  2. do not create ActiveModel::Serializer::Fieldset instance when fieldset is empty (optimization)
  3. check whether is fieldset falsy before using it

Caveats

NA

Related GitHub issues

Additional helpful information

bf4 and others added 30 commits January 5, 2017 21:32
Fix typos and capitalization in Relationship Links docs [ci skip]
Promote important architecture description that answers a lot of questions we get
Conflicts:
	docs/ARCHITECTURE.md
Link to 0.10.3 tag instead of `master` branch
Test was failing due to change in JSON exception message when parsing empty string
* delete KeyTransform, use CaseTransform

* added changelog

Conflicts:
	CHANGELOG.md
Update jsonapi runtime dependency to 0.1.1.beta6
Bump to v0.10.4 [ci skip]
Conflicts:
	CHANGELOG.md
* Updated isolated tests to assert correct behavior.
* Added check to get unsafe params if rails version is great than 5
…enderer_tests_a_bit

Cleanup assertions in isolated jsonapi renderer tests a bit
Add Model#attributes helper; make test attributes explicit
Fix relationship links doc
Conflicts:
	CHANGELOG.md
If the `id` attribute for a class isn't taken directly from the object when
serializing it, it may be desirible for other classes that serialize a
relationship with that class to overwrite the relationship IDs they serialize.

For example, suppose we have:

```(ruby)
class Repo < Model
  attributes :id, :github_id, :name
  associations :configs
end

class Config < Model
  attributes :id
  belongs_to :repo
end

class RepoSerializer < ActiveModel::Serializer
  attributes :id, :name

  has_many :update_configs

  def id
    object.github_id
  end
end

class ConfigSerializer < ActiveModel::Serializer
  attributes :id
  belongs_to :repo
end
```

In the above example, serializing a list of `Repo`s will give the `github_id`
for each one, but serializing a `Config` will give the `id` for its parent repo.

Ideally AMS would inspect the `RepoSerializer` when serializing the `Config`,
and realise it can't just output the foreign key. Unfortunately, getting the
serialization class for the child repo currently requires loading the record
(via evaluating `lazy_assocation`), and loses the performance benefit of the
existing `belongs_to?` path. Instead, I've opted to use
`read_attribute_for_serialization` instead of `object.send` to fetch the
serialized foreign key. This allows the serialized relationship ID to be
overwritten using

```(ruby)
class ConfigSerializer < ActiveModel::Serializer
  ...

  def repo_id
    object.repo.github_id
  end
end
```
f3z0 and others added 28 commits January 2, 2021 10:52
…ge is greater than total_pages

adding tests

fixing rubocop violation
Trigger github actions workflow on pull request
Handle edge case where requested current_page > total_pages in JSONAPI
[docs] disable logger via /dev/null instead of AS::N.unsubscribe
…nnot_infer_root_key_error_option

Add raise_cannot_infer_root_key_error to config
Relax gem requirement for Rails 7.0 prerelease
Relax gem requirement to allow Rails 7
The `Module#method_defined?` works for an instance method. But
`with_unbundled_env` defines as a class method. Therefore, the
current check doesn't work as expected.

```ruby
require "bundler"

puts Bundler.method_defined?(:with_unbundled_env) # => false
puts Bundler.respond_to?(:with_unbundled_env)     # => true
```

This fixes the following message that shows when running a test.

```
[DEPRECATED] `Bundler.with_clean_env` has been deprecated in favor of `Bundler.with_unbundled_env`. If you instead want the environment before bundler was originally loaded, use `Bundler.with_original_env` (called at /home/y-yagi/src/github.com/rails-api/active_model_serializers/Rakefile:68)
```
…method

Fix checking of method defined or not
…i#2370)

* refactor: instance_options[:fieldset] must be nil as it's not listed in ADAPTER_OPTION_KEYS
@liijunwei liijunwei closed this Aug 12, 2023
@liijunwei
Copy link
Author

liijunwei commented Aug 12, 2023

wrong target branch, pls ignore

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.

None yet