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.

Sign up for GitHub

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

Jump to bottom

Disable caching of batch connect form values #539

Merged
merged 31 commits into from
Aug 4, 2020
Merged

Disable caching of batch connect form values #539

merged 31 commits into from
Aug 4, 2020

Conversation

ericfranz
Copy link
Contributor

ericfranz
ericfranz commented Jun 25, 2020
View reviewed changes
apps/dashboard/app/models/batch_connect/session_context.rb Outdated
end

#Logic determining if attributes should be pulled from cache
def update_with_cache(cache, app_specific_cache)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The second argument app_specific_cache is not used and should be removed. Storing app_specific_cache_setting information on the session_context like you are doing makes the most sense because the app is what has the build method to create the session context

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess the problem I ran into here is that the form_config is a private method on app, the model has no way of accessing this, unless the controller shares it with the model correct ? Or maybe in the build_session_context in app.rb attach app_specific_cache_setting to the session_context model ?

@ericfranz ericfranz added this to the OOD1.8 milestone Jul 2, 2020
@Mreyna3 Mreyna3 marked this pull request as ready for review July 22, 2020 15:05
@ericfranz ericfranz merged commit 40a6b22 into master Aug 4, 2020
@ericfranz ericfranz deleted the disable_cache branch August 4, 2020 21:12
ericfranz added a commit that referenced this pull request Aug 4, 2020
@ericfranz
Revert "Disable caching of batch connect form values (#539)"
6241c47 
This reverts commit 40a6b22.
ericfranz added a commit that referenced this pull request Aug 4, 2020
@ericfranz @Mreyna3
Disable caching of batch connect form values (#539)
ad05070 
    Support disabling caching of app attributes

    1. disable globally all caching using OOD_BATCH_CONNECT_CACHE_ATTR_VALUES
    2. enable/disable caching per app by setting cacheable: true (or false)
    3. enable/disable caching per app attribute by setting cacheable: true (or false)

    As you can imagine, there are many permutations and those are captured in the tests.

**note: reverted and reverted again to correct attribution since Michaud did all the work**

Co-Authored-by: Michaud Reyna <Michaud.reyna14@gmail.com>
Co-Authored-By: mreyna3 <16469425+mreyna3@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johrstrom johrstrom johrstrom left review comments

@Mreyna3 Mreyna3 Mreyna3 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
OOD1.8
Development

Successfully merging this pull request may close these issues.
3 participants
@ericfranz @johrstrom @Mreyna3