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

Use report default options for Report screens #4749

Merged
merged 2 commits into from
Oct 16, 2018

Conversation

PanSpagetka
Copy link
Contributor

Reports on Save Report screen, should use default item count per page which is set in My Settings for Reports. @gtl_type variable used from deriving the per page count only can be grid, tile or list

Links [Optional]

https://bugzilla.redhat.com/show_bug.cgi?id=1616201

Steps for Testing/QA [Optional]

  1. Go to My Settings > Visual.
  2. Inside 'Default Items Per Page', set 'Reports' limit to anything except 20.
  3. Go to Cloud Intel > Reports > Saved Reports > All Saved Reports.
  4. If there aren't any saved reports, queue a report.
  5. On the All Saved Reports, check the paginator - value remains 20.

@PanSpagetka
Copy link
Contributor Author

@miq-bot add_label hammer/yes

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

What is your opinion about adding tests? 😉

@@ -1343,6 +1343,7 @@ def get_view_process_search_text(view)
end

def perpage_key(dbname)
return :reports if dbname == "miqreportresult"
Copy link
Member

Choose a reason for hiding this comment

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

I don't really like this 😥 could you at least convert it into a case?

@PanSpagetka PanSpagetka force-pushed the bz-1616201 branch 2 times, most recently from a77708c to a487960 Compare October 15, 2018 08:34
@PanSpagetka
Copy link
Contributor Author

ping @skateman

@PanSpagetka PanSpagetka force-pushed the bz-1616201 branch 3 times, most recently from 12527e2 to 5ca4233 Compare October 15, 2018 09:20
@miq-bot
Copy link
Member

miq-bot commented Oct 15, 2018

Checked commits PanSpagetka/manageiq-ui-classic@fb2ae5a~...5ca4233 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍪

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

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

The Seal of Approval

@mzazrivec mzazrivec self-assigned this Oct 16, 2018
@mzazrivec mzazrivec added this to the Sprint 97 Ending Oct 22, 2018 milestone Oct 16, 2018
@mzazrivec mzazrivec merged commit 7edf695 into ManageIQ:master Oct 16, 2018
simaishi pushed a commit that referenced this pull request Oct 16, 2018
Use report default options for Report screens

(cherry picked from commit 7edf695)

https://bugzilla.redhat.com/show_bug.cgi?id=1616201
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit e66bb6f1836fa8cc77026becebe4e9bf98a52acb
Author: Milan Zázrivec <[email protected]>
Date:   Tue Oct 16 11:29:11 2018 +0200

    Merge pull request #4749 from PanSpagetka/bz-1616201
    
    Use report default options for Report screens
    
    (cherry picked from commit 7edf69510d8984e533f9d7fab0e615854db6f28c)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1616201

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.

5 participants