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

fix: table cache #2734

Merged
merged 2 commits into from
May 1, 2024
Merged

fix: table cache #2734

merged 2 commits into from
May 1, 2024

Conversation

Paul-Bob
Copy link
Contributor

@Paul-Bob Paul-Bob commented May 1, 2024

Description

Fixes #2732

Refactor rows and header cache

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

@Paul-Bob Paul-Bob added the Fix label May 1, 2024
@Paul-Bob Paul-Bob self-assigned this May 1, 2024
Copy link

codeclimate bot commented May 1, 2024

Code Climate has analyzed commit 4e875ad and detected 0 issues on this pull request.

View more on Code Climate.

@Paul-Bob Paul-Bob merged commit 978cc5c into main May 1, 2024
17 of 20 checks passed
@Paul-Bob Paul-Bob deleted the fix/table_cache branch May 1, 2024 21:29
Copy link
Contributor

github-actions bot commented May 1, 2024

This PR has been merged into main. The functionality will be available in the next release.

Please check the release guide for more information.

@adrianthedev
Copy link
Collaborator

Could please leave a message to why this refactor was done?

@Paul-Bob
Copy link
Contributor Author

Paul-Bob commented May 2, 2024

Could please leave a message to why this refactor was done?

Sure, full context on the fixed issue: #2732

Instance variables was defined and assigned inside cache_if block which would not run if result already cached.

We actually want the cache_if block to cache a result and assign to instance variables every time that table component renders.

Pseudo code that explains the refactor:

Before:

cache_if ...
  @variable = value
end

After:

@variable = cache_if ...
  value
end

Makes sense?

@adrianthedev
Copy link
Collaborator

Got it! Thank you!

gabrielgiroe1 pushed a commit that referenced this pull request May 8, 2024
* fiix: table cache

* revert
Paul-Bob added a commit that referenced this pull request Aug 5, 2024
* Add support for alternative IDs using Hashids

* Fix rubocop

* Remove Hashid inclusion from ApplicationRecord

* Add  Hashid inclusion

* Fix error for hashid

* Fix lint error

* Add hashid-rails on development test

* Add require statement for hashid-rails gem in application.rb

* Remove unnecessary require statement for 'hashid/rails' in application.rb

* Update gemfile

* Resolve conflict
git reset

* Add gemfiles

* appraisal bundle

* Replace direct record id expouser with safer alternative

* Fix lint

* Revert to record.id

* Fix selector in tabs_spec

* Fix selector in has_many

* Fix selector

* Fix selector

* Fix selector

* Fix value for id

* wip

* Fix enum_array_display_value_false_spec

* possible quick fix for not showing belongs to

* Fix has_many_field_spec

* Fix fields_methods_for_views_spec

* fix: check proper dir for internal controller override (#2712)

* feature: configurable default pagination (#2717)

* feature: configurable default pagination

* add on template

* revert countless config

* feature: linkable association titles (#2591)

* fix: eager load actions (#2719)

* fix: actions eager loader (#2726)

* fix: render background context when accessing extrernal action links (#2730)

* fix: actions links POC

* wip

* TODO

* wip

* wip

* correct frame id

* lint

* lint

* fix: don't export `avo.custom.js` build (#2731)

* Bumped avo to 3.6.2

* fix: cache_table_rows (#2733)

* Bumped avo to 3.6.3

* fix: table cache (#2734)

* fiix: table cache

* revert

* Bumped avo to 3.6.4

* fix: resource controls spacing issue (#2740)

* fix: check if ActionText::RichText is defined (#2741)

* Bumped avo to 3.7.0

* chore(deps): bump trix in the npm_and_yarn group across 1 directory (#2749)

Bumps the npm_and_yarn group with 1 update in the / directory: [trix](https://github.com/basecamp/trix).


Updates `trix` from 2.1.0 to 2.1.1
- [Release notes](https://github.com/basecamp/trix/releases)
- [Commits](basecamp/trix@v2.1.0...v2.1.1)

---
updated-dependencies:
- dependency-name: trix
  dependency-type: direct:production
  dependency-group: npm_and_yarn
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore: require ostruct (#2750)

* [js] Update all Yarn dependencies (2024-04-25) (#2720)

Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com>
Co-authored-by: Paul Bob <69730720+Paul-Bob@users.noreply.github.com>
Co-authored-by: Adrian Marin <adrian@adrianthedev.com>

* wip

* feature: heading target (#2753)

* Update Gemfile

* Resolve conflict
git reset

* Update Gemfile

* Removed duplication

* Removed duplication

* wip

* wip

* wip

* tweaks for #2564 (#2776)

* tweaks

* .

* lint

* .

* test

* tweak

* revert tests on special branch

* wip

* wip

* wip

* wip

* wip

* Update lib/avo/fields/belongs_to_field.rb

* .

* belongs to field tweaks

* lint

* bundle

* fix belongs_to

* force lock

* bundle

* try --full-index

* appraisal update rubocop

* rm --full-index

* add record_param

* fix target_resource

* tweaks

* wip

* fix target_resource

* ransack cleanup

* dont include hashids for rails 8

* attach issue message

* fix belongs to selects

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Paul Bob <paul.ionut.bob@gmail.com>
Co-authored-by: Paul Bob <69730720+Paul-Bob@users.noreply.github.com>
Co-authored-by: Adrian Marin <adrian@adrianthedev.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: depfu[bot] <23717796+depfu[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resource index render fails with "#controller can't be used during initialization" error
2 participants