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

Remove unused Ansible Repository page refresh #4163

Conversation

Glutexo
Copy link
Member

@Glutexo Glutexo commented Jun 19, 2018

The refresh page button is no longer present on Ansible Repository Playbooks subpage. Because of that it is no longer necessary to handle pressing this button on this subpage. Remove this handling and related spec examples.

Steps to reproduce:

  1. Open an Ansible Repository show page.
  2. Press the leftmost Refresh button.

Now,

params[:display] = @display if @display
is executed, even though this is no longer necessary. This line was useful only for refreshing the Playbooks subpage.

Fixes #4160. Helps #3762.

@miq-bot add_reviewer @hstastna

Copy link
Contributor

@hstastna hstastna left a comment

Choose a reason for hiding this comment

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

It looks good, but I need to test it. I will let you know asap. Anyway, I would add this #2419 here just for info/explanation how params[:display] = @display if @display was added there some time ago. The problem there was that wrong toolbar was chosen while displaying nested list of playbooks - and this is why Refresh button was there (but did not work well, of course, and so there was effort to fix it - to make the button work).

@hstastna
Copy link
Contributor

hstastna commented Jun 19, 2018

So it looks ok to me and it works. @skateman could you check the changes for the tests, please? :) Thank you!

@skateman
Copy link
Member

Test changes are just deletions, seems okay, but travis is failing 😞

@Glutexo
Copy link
Member Author

Glutexo commented Jun 19, 2018

The failures and errors are from different specs and are unrelated. I can try rebasing if it helps.

The refresh page button is no longer present on Ansible Repository
Playbooks subpage. Because of that it is no longer necessary to handle
pressing this button on this subpage. Remove this handling and related
spec examples.
@Glutexo Glutexo force-pushed the remove_unused_ansible_repository_page_refresh branch from 724ddde to fafd853 Compare June 20, 2018 07:34
@miq-bot
Copy link
Member

miq-bot commented Jun 20, 2018

Checked commit Glutexo@fafd853 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. ⭐

@Glutexo
Copy link
Member Author

Glutexo commented Jun 20, 2018

I believe that at least here I am innocent. Travis is failing because of yarn timeout.

@skateman
Copy link
Member

@Glutexo probably yes, I will try something 😉

@mzazrivec mzazrivec self-assigned this Jun 20, 2018
@mzazrivec mzazrivec added this to the Sprint 89 Ending Jul 2, 2018 milestone Jun 20, 2018
@mzazrivec mzazrivec merged commit d422f08 into ManageIQ:master Jun 20, 2018
@Glutexo
Copy link
Member Author

Glutexo commented Jun 20, 2018

Just wondering, if the [technical-debt] flag means that the PR reduces the debt or increases it. 😅

@Glutexo Glutexo deleted the remove_unused_ansible_repository_page_refresh branch June 20, 2018 11:53
@mzazrivec
Copy link
Contributor

@Glutexo It means it reduces the tech. debt.

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.

None yet

5 participants