You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by tbouron <gi...@git.apache.org> on 2015/06/30 11:22:34 UTC

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

GitHub user tbouron opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/727

    [BROOKLYN-132] Add versions dropdown when creating applications from catalog template

    This PR adds a versions dropdown on the second step modal, when an application is created from a template. This dropdown is always visible for consistency, even if there is only one version available.
    
    Please note that as I was not sure that the `catalog-item` model could apply to any object returned by the `/v1/catalog/*` endpoints so I created a specific one for the applications only: `catalog-application`. It also makes sense as one backbone model should have only one endpoint.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/tbouron/incubator-brooklyn app-wizard

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-brooklyn/pull/727.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #727
    
----
commit 20bccfbda30c68399515d51d3e5db45713ebc545
Author: Thomas Bouron <th...@cloudsoftcorp.com>
Date:   2015-06-30T09:08:34Z

    [BROOKLYN-132] Add versions dropdown when creating applications from catalog template

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#discussion_r33557378
  
    --- Diff: usage/jsgui/src/main/webapp/assets/tpl/app-add-wizard/deploy-version-option.html ---
    @@ -0,0 +1,23 @@
    +
    +<!--
    +Licensed to the Apache Software Foundation (ASF) under one
    +or more contributor license agreements.  See the NOTICE file
    +distributed with this work for additional information
    +regarding copyright ownership.  The ASF licenses this file
    +to you under the Apache License, Version 2.0 (the
    +"License"); you may not use this file except in compliance
    +with the License.  You may obtain a copy of the License at
    +
    + http://www.apache.org/licenses/LICENSE-2.0
    +
    +Unless required by applicable law or agreed to in writing,
    +software distributed under the License is distributed on an
    +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +KIND, either express or implied.  See the License for the
    +specific language governing permissions and limitations
    +under the License.
    +-->
    +
    +<option value="<%= version %>">
    +    <span class="provider"><%= version %></span>
    --- End diff --
    
    Versions can come from humans so both of these should be escaped with `<%- .. %>`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-117186632
  
    @neykov @tbouron following from IRC discussion i like the idea that:
    
    * for multiple versions, show a drop down
    * for any single version, the user cannot interact but it *might* be useful to show the version
      * for a single version `0.0.0-SNAPSHOT` (the default) it isn't useful, so don't show anything
      * for any other single version, *do* show the version, for information
    
    the last one is optional in my mind but useful


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-117151540
  
    @sjcorbett A valuable one! @ahgittin, I guess you have the final say on that?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-117211217
  
    Done.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#discussion_r33557131
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/view/application-add-wizard.js ---
    @@ -105,15 +109,15 @@ define([
                                   step_id:'what-app',
                                   title:'Create Application',
                                   instructions:'Choose or build the application to deploy',
    -                              view:new ModalWizard.StepCreate({ model:this.model, wizard: this })
    +                              view:new ModalWizard.StepCreate({ model:this.model, wizard: this, catalog: this.catalog })
                               },
                               {
                                   // TODO rather than make this another step -- since we now on preview revert to the yaml tab
                                   // this should probably be shown in the catalog tab, replacing the other contents.
                                   step_id:'name-and-locations',
                                   title:'<%= appName %>',
    -                              instructions:'Specify the locations to deploy to and any additional configuration',
    -                              view:new ModalWizard.StepDeploy({ model:this.model })
    +                              instructions:'Specify the application version, locations to deploy to and any additional configuration',
    --- End diff --
    
    it's not the version of the actual *application* -- it's the version of the blueprint -- so this wording would be confusing.
    
    i suggest leave as it was, simplify -- we don't always show version, and if we do it's obvious enough what it is.  in time we could have a hover-help if it's an issue.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by alasdairhodge <gi...@git.apache.org>.
Github user alasdairhodge commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-120128549
  
    I'd suggest always including the version, even the uninteresting default. This communicates to the user that versioning is suppoprted (and encouraged), that a default is provided if none is specified, and reinforces the location of version info in the UI. It needn't be prominent, but should be present in a consistent place.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#discussion_r33557628
  
    --- Diff: usage/jsgui/src/main/webapp/assets/tpl/app-add-wizard/deploy-version-option.html ---
    @@ -0,0 +1,23 @@
    +
    +<!--
    +Licensed to the Apache Software Foundation (ASF) under one
    +or more contributor license agreements.  See the NOTICE file
    +distributed with this work for additional information
    +regarding copyright ownership.  The ASF licenses this file
    +to you under the Apache License, Version 2.0 (the
    +"License"); you may not use this file except in compliance
    +with the License.  You may obtain a copy of the License at
    +
    + http://www.apache.org/licenses/LICENSE-2.0
    +
    +Unless required by applicable law or agreed to in writing,
    +software distributed under the License is distributed on an
    +"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
    +KIND, either express or implied.  See the License for the
    +specific language governing permissions and limitations
    +under the License.
    +-->
    +
    +<option value="<%= version %>">
    +    <span class="provider"><%= version %></span>
    --- End diff --
    
    True that, my old JSP habit gets in a way.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-117113622
  
    @sjcorbett: I don't have any preference in that matter. But between having a disabled dropdown and no having one, I would prefer the second option. Does it make sense to add a `select` on this tiny modal if you cannot interact with it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-124024432
  
    @neykov: I tried again my branch https://github.com/tbouron/incubator-brooklyn/tree/app-wizard and it works as expected. Are you sure you checked out the correct branch?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#discussion_r33557476
  
    --- Diff: usage/jsgui/src/main/webapp/assets/js/view/application-add-wizard.js ---
    @@ -105,15 +109,15 @@ define([
                                   step_id:'what-app',
                                   title:'Create Application',
                                   instructions:'Choose or build the application to deploy',
    -                              view:new ModalWizard.StepCreate({ model:this.model, wizard: this })
    +                              view:new ModalWizard.StepCreate({ model:this.model, wizard: this, catalog: this.catalog })
                               },
                               {
                                   // TODO rather than make this another step -- since we now on preview revert to the yaml tab
                                   // this should probably be shown in the catalog tab, replacing the other contents.
                                   step_id:'name-and-locations',
                                   title:'<%= appName %>',
    -                              instructions:'Specify the locations to deploy to and any additional configuration',
    -                              view:new ModalWizard.StepDeploy({ model:this.model })
    +                              instructions:'Specify the application version, locations to deploy to and any additional configuration',
    --- End diff --
    
    True, I'll revert that to what it was.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-117098787
  
    good fix -- showing a tile for each version in the wizard was awful
    
    however i don't like adding dropdowns etc unless we really need them.  arguably the wizard interface never asks you (use yaml if you want a non-standard version) -- but anything is an improvement over the one-tile-per-version!
    
    can we make the dropdown so it only shows if there is more than one (that should be easy, no?), and it's populated with the default?
    
    also note that the default is the highest **non-snapshot** version -- `CatalogItemComparator` sorts this, so you should be able just to take the first in the list.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-124165147
  
    Commit squashed


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-117111059
  
    To bikeshed a little, how about disabling the dropdown rather than hiding it when there's only one entry?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-117147463
  
    Personally I think it's more consistent to always keep the element. Will users understand why it has disappeared? Anyway, just an opinion. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-117109103
  
    @ahgittin The version sorting is already done as per as what you want. Here is an example:
    
    ![screen shot 2015-06-30 at 10 49 26](https://cloud.githubusercontent.com/assets/2082759/8428613/d396f488-1f17-11e5-8651-afa2334600a6.png)
    
    I can easily hide the dropdown if there is only version. I'll push these changes ASAP



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-117156709
  
    most people aren't using versions and it would confuse/distract them so i think better to hide it in that vast majority case
    
    ps - @tbouron i don't have any special say in the matter


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/727


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-119906313
  
    Tried to test the functionality, but couldn't see the version selector.
    In the app selection window the template appears, tagged with the latest version:
    <img width="575" alt="screen shot 2015-07-09 at 13 36 52" src="https://cloud.githubusercontent.com/assets/3612111/8593375/da5d77cc-263f-11e5-96b9-71114eff7b05.png">
    
    After selecting the template next window doesn't have version info:
    <img width="595" alt="screen shot 2015-07-09 at 13 37 01" src="https://cloud.githubusercontent.com/assets/3612111/8593384/e7cd37c6-263f-11e5-848c-1795f85ffea2.png">
    
    Once done either squash the commits or add subjects/description to each logically separate commit.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-117196239
  
    @ahgittin I really like this idea too. I'll make the changes and commit ASAP


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-124045671
  
    Looks like I got bitten by https://github.com/apache/incubator-brooklyn/pull/753. After a handful of rebuilds I can finally see the dropdown.
    
    Good to merge, but better squash the commits first (see comment from above).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-brooklyn pull request: [BROOKLYN-132] Add versions dropd...

Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/727#issuecomment-124335753
  
    Thanks @tbouron - and thanks to all the reviewers! Merging now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---