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 undefined method reject! in allowed_storages #22636

Merged
merged 1 commit into from
Jul 25, 2023

Conversation

agrare
Copy link
Member

@agrare agrare commented Jul 25, 2023

In the case where a StorageProfile is selected we were failing to filter out the allowed storages due to the storages variable now being an ActiveRecord Association where #reject! isn't defined.

This also resolves an N+1 issue where each storage queries for all storage_profiles.

Failing spec: https://github.com/ManageIQ/manageiq-content/actions/runs/5651086179/job/15308573621

Related: #22625

@agrare agrare added the bug label Jul 25, 2023
@agrare
Copy link
Member Author

agrare commented Jul 25, 2023

@miq-bot cross-repo-tests manageiq-content

@Fryguy
Copy link
Member

Fryguy commented Jul 25, 2023

LGTM - would it be possible to get some specs for these cases into the core repo?

@Fryguy Fryguy self-assigned this Jul 25, 2023
@agrare agrare force-pushed the fix_allowed_storages_storage_profile branch 2 times, most recently from c26ae68 to 5a49403 Compare July 25, 2023 20:21
In the case where a StorageProfile is selected we were failing to filter
out the allowed storages due to the `storages` variable now being an
ActiveRecord Association where `#reject!` isn't defined.

This also resolves an N+1 issue where each storage queries for all
storage_profiles.
@agrare agrare force-pushed the fix_allowed_storages_storage_profile branch from 5a49403 to cee957b Compare July 25, 2023 20:24
@miq-bot
Copy link
Member

miq-bot commented Jul 25, 2023

Checked commit agrare@cee957b with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
2 files checked, 0 offenses detected
Everything looks fine. 🏆

@bdunne bdunne merged commit b64767d into ManageIQ:master Jul 25, 2023
9 checks passed
@bdunne bdunne assigned bdunne and unassigned Fryguy Jul 25, 2023
@agrare agrare deleted the fix_allowed_storages_storage_profile branch July 25, 2023 21:29
@Fryguy
Copy link
Member

Fryguy commented Jul 25, 2023

Backported to petrosian in commit 9e73915.

commit 9e73915a8a086793d2e3b40ccaffaf135c7a3c9a
Author: Brandon Dunne <brandondunne@hotmail.com>
Date:   Tue Jul 25 17:17:56 2023 -0400

    Merge pull request #22636 from agrare/fix_allowed_storages_storage_profile
    
    Fix undefined method reject! in allowed_storages
    
    (cherry picked from commit b64767d37bf32be5877cd5327950fd9421b18776)

Fryguy pushed a commit that referenced this pull request Jul 25, 2023
…ofile

Fix undefined method reject! in allowed_storages

(cherry picked from commit b64767d)
@Fryguy
Copy link
Member

Fryguy commented Jul 26, 2023

Backported to morphy in commit f71e932.

commit f71e932f34454d2b0c38c3824e41e21152e0bbb2
Author: Brandon Dunne <brandondunne@hotmail.com>
Date:   Tue Jul 25 17:17:56 2023 -0400

    Merge pull request #22636 from agrare/fix_allowed_storages_storage_profile
    
    Fix undefined method reject! in allowed_storages
    
    (cherry picked from commit b64767d37bf32be5877cd5327950fd9421b18776)

Fryguy pushed a commit that referenced this pull request Jul 26, 2023
…ofile

Fix undefined method reject! in allowed_storages

(cherry picked from commit b64767d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants