UI for server group deployments #23

jkremser · 2016-12-23T14:10:08Z

This PR adds the ability to deploy a war file into whole server group and also prepares the ground for the following operations on the server group like starting, stopping, etc.

A lot of the functionality was common with deploying into servers. Therefore the (angular) js controllers are now used in both contexts. Also the MiddlewareDeploymentsMixin was introduced so that MiddlewareServerGroupController and MiddlewareServerController don't have to repeat the code.

This shouldn't be merged (or released) sooner than this PR lands into manageiq repo.

todo:

reuse the existing haml file for deploying into servers

@miq-bot add_labels providers/hawkular, enhancement, euwe/no

miq-bot · 2016-12-23T14:16:29Z

@Jiri-Kremser Cannot apply the following label because they are not recognized: providers/hawkular

jkremser · 2016-12-23T14:41:51Z

10 files checked, 72 offenses detected
ho ho ho 🎅

jkremser · 2016-12-23T20:47:42Z

@miq-bot add_labels wip

jkremser · 2017-01-04T11:35:43Z

@miq-bot remove_labels wip

jkremser · 2017-01-05T11:35:50Z

@miq-bot add_labels pending_backend

jkremser · 2017-01-19T15:02:54Z

@miq-bot remove_labels pending_backend

jkremser · 2017-01-19T15:04:51Z

The backend part has been merged.

note: this was check that it works ManageIQ/manageiq#13173 (comment)

mzazrivec · 2017-01-23T15:10:48Z

app/assets/javascripts/controllers/middleware_deployment/middleware_deployment_controller.js

@@ -21,7 +23,8 @@ function MwAddDeploymentController($scope, $http, miqService) {
          miqService.miqFlash(result.data.status, result.data.msg);
        },
        function() { // error
-          miqService.miqFlash('error', 'Unable to deploy "' + data.runtimeName + '" on this server.');
+          miqService.miqFlash('error', 'Unable to deploy "' + data.runtimeName + '" on this server' +
+              (isGroupDeployment ? ' group' : '') +'.');


The content of the miqFlash() here needs to be inside a __() so that it's correctly translated. Since this
is a dynamic string, you also need to use sprintf(_("Whatever %s %s"), data..., data...)

mzazrivec · 2017-01-23T15:17:47Z

app/controllers/mixins/middleware_deployments_mixin.rb

+    if params['forceDeploy'] == 'false'
+      existing_deployment = find_existing_deployment
+      unless existing_deployment.nil?
+        msg = 'Deployment "%s" already exists on this ' + parent_entity_name


This isn't going to work with gettext. You need to use

msg = _("Deployment \"%{deployment_name}\" already exists on this %{parent}") % {:deployment_name => ..., :parent => ...}

Also, in general, it is a bad practice to put two english strings together and expect it to work in other locales.
In the end, the two translated strings put together will just look odd. In this case, I suggest we have:

msg = if @is_server _("Full string %{some_placeholder} end of the string.") else _("The other variant %{some_placeholder} the other end of the string.") end

Yeah, makes sense from the linguistic POV. But isn't it too strict? With this reasoning, we should get rid of all those strings that have more than one variable in it (other than name or id).

I did the change as you proposed, but it looks odd in the code.

mzazrivec · 2017-01-23T15:17:55Z

app/controllers/mixins/middleware_deployments_mixin.rb

+
+  def run_it(operation)
+    run_specific_operation(@klass.operations.fetch(operation), @entity_id)
+    msg = 'Deployment "%s" has been initiated on this ' + parent_entity_name


Same as above.

mzazrivec · 2017-01-23T15:37:03Z

app/controllers/middleware_server_group_controller.rb


  before_action :check_privileges
  before_action :get_session_data
  after_action :cleanup_action
  after_action :set_session_data

+  OPERATIONS = {
+    :middleware_server_group_reload         => {:op  => :reload_middleware_server_group,
+                                                :msg => N_('Reload')


I'm getting a bit lost (not saying the code above is incorrect). Where is _() being applied to the :msg strings here?

I am not the author of the concept, but afaik these are actually the strings that get plugged into sentences. Here is an example where it's being used https://git.io/vMbLs

One more thing -- if that's what it is, then in

add_flash(_("%{operation} initiated for selected server(s)") % {:operation => operation_info.fetch(:msg)})

you need to apply _() on operation_info.fetch(:msg) otherwise the operation string
won't be translated.

miq-bot · 2017-02-01T15:38:24Z

Checked commits jkremser/manageiq-ui-classic@f9cbd64~...2358fbc with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
8 files checked, 0 offenses detected
Everything looks good. 🍪

…-deployments" This reverts commit 276f397, reversing changes made to d92ebc3.

jkremser mentioned this pull request Dec 23, 2016

Server group deployments ManageIQ/manageiq#13173

Merged

3 tasks

miq-bot added enhancement euwe/no labels Dec 23, 2016

miq-bot added the wip label Dec 23, 2016

jkremser changed the title ~~[WIP] UI for server group deployments~~ UI for server group deployments Jan 4, 2017

jkremser force-pushed the server-group-deployments branch from df5f0aa to 2e41b79 Compare January 4, 2017 16:22

miq-bot removed the wip label Jan 5, 2017

miq-bot added the pending core label Jan 5, 2017

miq-bot removed the pending core label Jan 19, 2017

mzazrivec suggested changes Jan 23, 2017

View reviewed changes

mzazrivec self-assigned this Jan 23, 2017

jkremser force-pushed the server-group-deployments branch from 2e41b79 to f171b21 Compare January 24, 2017 11:04

jkremser added 5 commits February 1, 2017 16:26

View part for server group deployments.

f9cbd64

Adding 2 js controllers for server group deployments.

077874a

Controllers part for server group deployments.

69a14cb

Reusing one haml in two different context.

c254370

Fixing the i18n strings.

c07523c

jkremser force-pushed the server-group-deployments branch from f171b21 to dc1528b Compare February 1, 2017 15:29

Rubocop&haml_lint

2358fbc

jkremser force-pushed the server-group-deployments branch from dc1528b to 2358fbc Compare February 1, 2017 15:37

mzazrivec approved these changes Feb 2, 2017

View reviewed changes

mzazrivec added this to the Sprint 54 Ending Feb 13, 2017 milestone Feb 2, 2017

mzazrivec merged commit 276f397 into ManageIQ:master Feb 2, 2017

jkremser deleted the server-group-deployments branch February 2, 2017 12:54

israel-hdez added a commit to israel-hdez/manageiq-ui-classic that referenced this pull request Jan 5, 2018

Revert "Merge pull request ManageIQ#23 from Jiri-Kremser/server-group…

502b41a

…-deployments" This reverts commit 276f397, reversing changes made to d92ebc3.

israel-hdez added a commit to israel-hdez/manageiq-ui-classic that referenced this pull request Jan 11, 2018

Revert "Merge pull request ManageIQ#23 from Jiri-Kremser/server-group…

b44347c

…-deployments" This reverts commit 276f397, reversing changes made to d92ebc3.

israel-hdez added a commit to israel-hdez/manageiq-ui-classic that referenced this pull request Jan 15, 2018

Revert "Merge pull request ManageIQ#23 from Jiri-Kremser/server-group…

e85f16a

…-deployments" This reverts commit 276f397, reversing changes made to d92ebc3.

israel-hdez added a commit to israel-hdez/manageiq-ui-classic that referenced this pull request Jan 17, 2018

Revert "Merge pull request ManageIQ#23 from Jiri-Kremser/server-group…

da5fcc9

…-deployments" This reverts commit 276f397, reversing changes made to d92ebc3.

israel-hdez added a commit to israel-hdez/manageiq-ui-classic that referenced this pull request Jan 19, 2018

Revert "Merge pull request ManageIQ#23 from Jiri-Kremser/server-group…

89910e4

…-deployments" This reverts commit 276f397, reversing changes made to d92ebc3.

israel-hdez added a commit to israel-hdez/manageiq-ui-classic that referenced this pull request Jan 22, 2018

Revert "Merge pull request ManageIQ#23 from Jiri-Kremser/server-group…

3e08822

…-deployments" This reverts commit 276f397, reversing changes made to d92ebc3.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI for server group deployments #23

UI for server group deployments #23

jkremser commented Dec 23, 2016 •

edited

Loading

miq-bot commented Dec 23, 2016

jkremser commented Dec 23, 2016

jkremser commented Dec 23, 2016

jkremser commented Jan 4, 2017

jkremser commented Jan 5, 2017

jkremser commented Jan 19, 2017

jkremser commented Jan 19, 2017

mzazrivec Jan 23, 2017

mzazrivec Jan 23, 2017

mzazrivec Jan 23, 2017

jkremser Jan 24, 2017

mzazrivec Jan 23, 2017

mzazrivec Jan 23, 2017

jkremser Jan 24, 2017 •

edited

Loading

mzazrivec Feb 2, 2017

miq-bot commented Feb 1, 2017

UI for server group deployments #23

UI for server group deployments #23

Conversation

jkremser commented Dec 23, 2016 • edited Loading

miq-bot commented Dec 23, 2016

jkremser commented Dec 23, 2016

jkremser commented Dec 23, 2016

jkremser commented Jan 4, 2017

jkremser commented Jan 5, 2017

jkremser commented Jan 19, 2017

jkremser commented Jan 19, 2017

mzazrivec Jan 23, 2017

Choose a reason for hiding this comment

mzazrivec Jan 23, 2017

Choose a reason for hiding this comment

mzazrivec Jan 23, 2017

Choose a reason for hiding this comment

jkremser Jan 24, 2017

Choose a reason for hiding this comment

mzazrivec Jan 23, 2017

Choose a reason for hiding this comment

mzazrivec Jan 23, 2017

Choose a reason for hiding this comment

jkremser Jan 24, 2017 • edited Loading

Choose a reason for hiding this comment

mzazrivec Feb 2, 2017

Choose a reason for hiding this comment

miq-bot commented Feb 1, 2017

jkremser commented Dec 23, 2016 •

edited

Loading

jkremser Jan 24, 2017 •

edited

Loading