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

improve logic and ui for inferring bundle and type name when saving to catalog #108

Merged
merged 3 commits into from Nov 13, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -44,6 +44,7 @@ const TYPES = [
angular.module(MODULE_NAME, [angularAnimate, uibModal, brUtils])
.directive('catalogSaver', ['$rootScope', '$uibModal', '$injector', 'composerOverrides', 'blueprintService', saveToCatalogModalDirective])
.directive('catalogVersion', ['$parse', catalogVersionDirective])
.filter('bundlize', bundlizeProvider)
.run(['$templateCache', templateCache]);

export default MODULE_NAME;
Expand Down Expand Up @@ -87,10 +88,9 @@ export function saveToCatalogModalDirective($rootScope, $uibModal, $injector, co
$scope.config.symbolicName = entity.id || metadata.get('id');
}
if (!$scope.config.bundle) {
let bundle = $scope.config.symbolicName || $scope.config.name || 'untitled';
$scope.config.bundle = bundle.split(/[^-a-zA-Z0-9._]+/).join('-').toLowerCase();
if (!$scope.config.symbolicName) {
$scope.config.symbolicName = $scope.config.bundle;
// NB: when editing a bundle this will already be set
if ($scope.config.symbolicName) {
$scope.config.bundle = $scope.config.symbolicName;
}
}

Expand Down Expand Up @@ -133,9 +133,9 @@ export function CatalogItemModalController($scope, blueprintService, paletteApi,
$scope.getTitle = () => {
switch ($scope.state.view) {
case VIEWS.form:
return $scope.isUpdate ? `Update ${$scope.config.name || $scope.config.symbolicName}` : 'Add to catalog';
return $scope.isUpdate ? `Update ${$scope.config.name || $scope.config.symbolicName || 'blueprint'}` : 'Add to catalog';
case VIEWS.saved:
return `${$scope.config.name || $scope.config.symbolicName} ${$scope.isUpdate ? 'updated' : 'saved'}`;
return `${$scope.config.name || $scope.config.symbolicName || 'Blueprint'} ${$scope.isUpdate ? 'updated' : 'saved'}`;
}
};

Expand All @@ -160,11 +160,22 @@ export function CatalogItemModalController($scope, blueprintService, paletteApi,
function createBom() {
let blueprint = blueprintService.getAsJson();

let bundleBase = $scope.config.bundle || bundlize($scope.config.name);
let bundleId = $scope.config.symbolicName || bundlize($scope.config.name);
if (!bundleBase || !bundleId) {
throw "Either display name must be set of bundle and symbolic name explicitly set";
}

let bomItem = {
id: $scope.config.symbolicName,
id: bundleId,
itemType: $scope.config.itemType,
item: blueprint
};
let bomCatalogYaml = {
bundle: `catalog-bom-${bundleBase}`,
version: $scope.config.version,
items: [ bomItem ]
};
if (brUtilsGeneral.isNonEmpty($scope.config.name)) {
bomItem.name = $scope.config.name;
}
Expand All @@ -174,14 +185,8 @@ export function CatalogItemModalController($scope, blueprintService, paletteApi,
if (brUtilsGeneral.isNonEmpty($scope.config.iconUrl)) {
bomItem.iconUrl = $scope.config.iconUrl;
}

return jsYaml.dump({
'brooklyn.catalog': {
bundle: `catalog-bom-${$scope.config.bundle}`,
version: $scope.config.version,
items: [bomItem]
}
});

return jsYaml.dump({ 'brooklyn.catalog': bomCatalogYaml });
}
}

Expand Down Expand Up @@ -223,3 +228,7 @@ function templateCache($templateCache) {
$templateCache.put(TEMPLATE_URL, template);
$templateCache.put(TEMPLATE_MODAL_URL, modalTemplate);
}

var bundlize = (input) => input && input.split(/[^a-zA-Z0-9]+/).filter(x => x).join('-').toLowerCase();
Copy link
Member

@tbouron tbouron Nov 13, 2018

Choose a reason for hiding this comment

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

Why create this var (which should be let) ? The function should live within bundlizeProvider which you can then call directly within this file with bundlizeProvider()(input);. Actually, a better way would be to rely on the $filter service like so: $filter('bundlize')(input);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i didn't like bundlizeProvider()(textToBundlize), but $filter('bundlize')(text) is not bad, changed

function bundlizeProvider() { return bundlize; }

Expand Up @@ -24,9 +24,12 @@ <h3 class="modal-title">{{getTitle()}}</h3>
<div class="modal-body add-to-catalog-modal">
<form ng-show="state.view === VIEWS.form" name="form" novalidate>

<div class="form-group">
<div class="form-group" ng-class="{'has-error': form.name.$invalid}">
<label class="control-label">Blueprint display name</label>
<input ng-model="config.name" ng-disabled="state.saving" class="form-control" name="name" type="text" />
<input ng-model="config.name" ng-disabled="state.saving" class="form-control" name="name" type="text" required />
<p class="help-block" ng-show="form.name.$invalid">
<span ng-if="form.name.$error.required">You must specify a name for this item</span>
</p>
</div>

<div class="form-group">
Expand Down Expand Up @@ -57,19 +60,17 @@ <h3 class="modal-title">{{getTitle()}}</h3>
<label class="control-label">Bundle ID</label>
<div class="input-group">
<span class="input-group-addon">catalog-bom-</span>
<input ng-model="config.bundle" ng-disabled="state.saving" class="form-control" placeholder="E.g my-bundle" name="bundle" required ng-pattern="state.pattern" autofocus />
<input ng-model="config.bundle" ng-disabled="state.saving" class="form-control" name="bundle" ng-pattern="state.pattern" autofocus placeholder="{{ config.name | bundlize }}"/>
Copy link
Member

Choose a reason for hiding this comment

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

I get that config.name is required but until you set it, there will be no placeholder. So you should do {{(config.name | bundlize) || 'E.g my-bundle'}}

</div>
<p class="help-block" ng-show="form.bundle.$invalid">
<span ng-if="form.bundle.$error.required">You must specify a bundle ID</span>
<span ng-if="form.bundle.$error.pattern">The bundle ID can contains only letters, numbers as well a the following characters: <code>.</code>, <code>-</code> and <code>_</code></span>
</p>
</div>

<div class="form-group" ng-class="{'has-error': form.symbolicName.$invalid}">
<label class="control-label">Blueprint symbolic name</label>
<input ng-model="config.symbolicName" ng-disabled="state.saving" class="form-control" placeholder="E.g my-catalog-id" name="symbolicName" required ng-pattern="state.pattern" autofocus />
<input ng-model="config.symbolicName" ng-disabled="state.saving" class="form-control" name="symbolicName" ng-pattern="state.pattern" autofocus placeholder="{{ config.name | bundlize }}"/>
Copy link
Member

Choose a reason for hiding this comment

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

I get that config.name is required but until you set it, there will be no placeholder. So you should do {{(config.name | bundlize) || 'E.g my-catalog-id'}}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good spot, fitxed

<p class="help-block" ng-show="form.symbolicName.$invalid">
<span ng-if="form.symbolicName.$error.required">You must specify a blueprint symbolic name</span>
<span ng-if="form.symbolicName.$error.pattern">The blueprint symbolic name can contains only letters, numbers as well a the following characters: <code>.</code>, <code>-</code> and <code>_</code></span>
</p>
</div>
Expand Down