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

Add reload button to Ansible Repositories #1366

Merged
merged 1 commit into from
May 29, 2017

Conversation

ZitaNemeckova
Copy link
Contributor

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

Ansible Repositories:
screen shot 2017-05-17 at 2 24 38 pm

Ansible Repository:
screen shot 2017-05-17 at 2 24 28 pm

@miq-bot add_label bug, automation/ansible

@ZitaNemeckova
Copy link
Contributor Author

@mzazrivec please review, thanks :)

@miq-bot
Copy link
Member

miq-bot commented May 17, 2017

Checked commit ZitaNemeckova@cd5a144 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 8 offenses detected

app/helpers/application_helper/toolbar/ansible_repositories_center.rb

  • ❗ - Line 10, Col 3 - Style/IndentArray - Indent the right bracket the same as the first position after the preceding left parenthesis.
  • ❗ - Line 3, Col 5 - Style/IndentArray - Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.
  • ❗ - Line 9, Col 66 - Style/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
  • ❗ - Line 9, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.

app/helpers/application_helper/toolbar/ansible_repository_center.rb

  • ❗ - Line 10, Col 3 - Style/IndentArray - Indent the right bracket the same as the first position after the preceding left parenthesis.
  • ❗ - Line 3, Col 5 - Style/IndentArray - Use 2 spaces for indentation in an array, relative to the first position after the preceding left parenthesis.
  • ❗ - Line 9, Col 66 - Style/MultilineMethodCallBraceLayout - Closing method call brace must be on the line after the last argument when opening brace is on a separate line from the first argument.
  • ❗ - Line 9, Col 7 - Style/AlignHash - Align the elements of a hash literal if they span more than one line.


def check_button_rbac
# Allow reload to skip RBAC check
if %w(ansible_repository_reload ansible_repositories_reload).include?(params[:pressed])
Copy link
Contributor

Choose a reason for hiding this comment

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

Cc @martinpovolny , @PanSpagetka , @romanblanco ... it looks like we have to do this for any ButtonWithoutRbacCheck because the default check_button_rbac doesn't read the toolbar def and doesn't know a check is not needed.

But it feels like this should be easier...

@ZitaNemeckova
Copy link
Contributor Author

Broken by #1323.

@miq-bot add_label wip

@miq-bot miq-bot changed the title Add reload button to Ansible Repositories [WIP] Add reload button to Ansible Repositories May 18, 2017
@miq-bot miq-bot added the wip label May 18, 2017
@ZitaNemeckova
Copy link
Contributor Author

Waiting on #1378

@martinpovolny
Copy link
Member

So for post-fine the GTL reload button should be javascript based, right?

Is there something needed from the GTL component (@karelhala) to make that happen or do we have all the bits?

@martinpovolny
Copy link
Member

I also wonder about TextualSummary reload being javascript based. We can quickly add such functionality to the GenericShowMixin or shall we better have a TextualSummary Angular component first? What do you think @himdel ?

@ZitaNemeckova
Copy link
Contributor Author

@miq-bot remove_label wip

@miq-bot miq-bot changed the title [WIP] Add reload button to Ansible Repositories Add reload button to Ansible Repositories May 22, 2017
@miq-bot miq-bot removed the wip label May 22, 2017
@karelhala
Copy link
Contributor

JS refresh would be really great for both textualSummary and for GTL components. I guess moving TextualSummary to angular would be the first step.

@ZitaNemeckova if you want to refresh GTL via JS you can use this diff

diff --git a/app/assets/javascripts/controllers/report_data_controller.js b/app/assets/javascripts/controllers/report_data_controller.js
index e3a3712..57421a3 100644
--- a/app/assets/javascripts/controllers/report_data_controller.js
+++ b/app/assets/javascripts/controllers/report_data_controller.js
@@ -86,6 +86,8 @@
         this.onUnsubscribe();
       } else if (event.tollbarEvent && (event.tollbarEvent === 'itemClicked')) {
         this.setExtraClasses();
+      } else if (event.refreshController && event.refreshController.name === COTNROLLER_NAME) {
+        this.initController(this.initObject);
       }
 
       if (event.controller === COTNROLLER_NAME && this.apiFunctions && this.apiFunctions[event.action]) {

And then call it:

sendDatawithRx({refreshController: {name: 'reportDataController'}});

No need to follow the exact API, just proposition, however we could use refreshController event in some other parts of appliaction hence passing the name inside object.

@himdel
Copy link
Contributor

himdel commented May 22, 2017

Maybe we should move that discussion to the relevant issue: #1365 .

(So, Karel, agreed, pretty much what I proposed there :).)

@mzazrivec mzazrivec self-assigned this May 29, 2017
@mzazrivec mzazrivec added this to the Sprint 62 Ending Jun 5, 2017 milestone May 29, 2017
@mzazrivec mzazrivec merged commit 3c001ab into ManageIQ:master May 29, 2017
@ZitaNemeckova ZitaNemeckova deleted the repos_refresh_button branch September 12, 2017 14:55
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.

6 participants