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.

Sign up for GitHub

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

Jump to bottom

Fixes #23481 - Host Collection Action Modal Links #7353

Merged
merged 1 commit into from
Jun 7, 2018
Merged

Fixes #23481 - Host Collection Action Modal Links #7353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 10 additions & 3 deletions ...ascripts/bastion_katello/content-hosts/bulk/content-hosts-bulk-errata-modal.controller.js
View file
Open in desktop
Expand Up @@ -40,7 +40,8 @@ angular.module('Bastion.content-hosts').controller('ContentHostsBulkErrataModalC
nutupane = new Nutupane(HostBulkAction, hostIds, 'installableErrata');
$scope.controllerName = 'katello_errata';
nutupane.masterOnly = true;

$scope.showErrata = false;
$scope.showHosts = false;
$scope.table = nutupane.table;
$scope.table.errataFilterTerm = "";
$scope.table.initialLoad = false;
Expand All @@ -57,6 +58,10 @@ angular.module('Bastion.content-hosts').controller('ContentHostsBulkErrataModalC
$scope.errataActionFormValues.hostIds = hostIds.included.ids.join(',');
}

$scope.showTable = function () {
return (!$scope.showErrata && !$scope.showHosts);
};

$scope.fetchErrata = function () {
var params = hostIds;
params['organization_id'] = CurrentOrganization;
Expand All @@ -73,14 +78,16 @@ angular.module('Bastion.content-hosts').controller('ContentHostsBulkErrataModalC

$scope.transitionToErrata = function (erratum) {
fetchErratum(erratum['errata_id']);
$scope.transitionTo('content-hosts.bulk-actions.errata.details', {errataId: erratum['errata_id']});
$scope.erratum = erratum;
$scope.showErrata = true;
};

$scope.transitionToErrataContentHosts = function (erratum) {
$scope.erratum = erratum;
$scope.transitionTo('content-hosts.bulk-actions.errata.content-hosts', {errataId: erratum['errata_id']});
$scope.showHosts = true;
};


$scope.installErrata = function () {
if ($scope.remoteExecutionByDefault) {
$scope.installErrataViaRemoteExecution();
Expand Down
1 change: 0 additions & 1 deletion ...astion_katello/content-hosts/bulk/content-hosts-bulk-host-collections-modal.controller.js
View file
Open in desktop
Expand Up @@ -38,7 +38,6 @@ angular.module('Bastion.content-hosts').controller('ContentHostsBulkHostCollecti
nutupane.masterOnly = true;

$scope.table = nutupane.table;

$scope.confirmHostCollectionAction = function (action) {
$scope.hostCollections.confirm = true;
$scope.hostCollections.action = action;
Expand Down
99 changes: 98 additions & 1 deletion ...javascripts/bastion_katello/content-hosts/bulk/views/content-hosts-bulk-errata-modal.html
View file
Open in desktop
Expand Up @@ -27,7 +27,7 @@ <h4 data-block="modal-header">Content Host Errata Management</h4>
</div>
</div>

<div data-extend-template="layouts/partials/table.html">
<div ng-show="showTable()" data-extend-template="layouts/partials/table.html">
<div data-block="search">
<input type="text" class="form-control" stop-event="click"
placeholder="{{ 'Filter...' | translate }}"
Expand Down Expand Up @@ -105,6 +105,103 @@ <h4 data-block="modal-header">Content Host Errata Management</h4>
</table>
</div>
</div>

<div name="errataDetails" ng-show="showErrata">
Copy link
Member

Choose a reason for hiding this comment

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

I'm unclear why this is a better solution than simply linking to the erratum details page. If we really want to use modals, at least we could use a partial to reduce the risk of introducing bugs if something (i.e. the API) changes in the future. From a development point of view, it's just not DRY. Also, the erratum details page lists the repositories and content hosts affected as well, which I think (the repos) is useful information not currently provided by these modals.

I can see an argument for the modal in that it decreases context switching for our users. Maybe they just want to apply any applicable errata but want to just 'peek' at a few errata to see which packages are provided, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

@akofink : So this modal logic with link redirects had these 2 pages Errata Content host and Errata Details which become dead code following this change.

These 2 pages worked fine till 6.2 but once we moved to modals in 6.3, the redirects stopped working.

Do you suggest turning these pages into modals or a simple redirect to the regular errata detail page etc..(We still have some context switching involved with links on the other modals. I don't know why errata was treated differently in the first place)

Copy link
Member

Choose a reason for hiding this comment

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

Hmmm... I guess we should fix it to work as was intended in 3.4 (6.3) with the modals? It's kind of a UX question, so maybe @Rohoover has some input. We should definitely still support all the workflows we used to support in 3.0 (6.2) before we changed over to modals.

<a ng-click="showErrata = false">
<i class="fa fa-double-angle-left"></i>
{{ "Back To Errata List" | translate }}
</a>

<h3>{{ 'Errata Details' | translate }}</h3>

<h3>{{ erratum.errata_id }} </h3>

<div class="details details-full">
<section>

<div class="detail">
<span class="info-label" translate>Type</span>
<span class="info-value">{{ erratum.type }}</span>
</div>
<div class="detail">
<span class="info-label" translate>Title</span>
<span class="info-value">{{ erratum.title }}</span>
</div>
<div class="detail">
<span class="info-label" translate>Issued</span>
<span class="info-value">{{ erratum.issued }}</span>
</div>
<div class="detail">
<span class="info-label" translate>Updated</span>
<span class="info-value">{{ erratum.updated }}</span>
</div>
<div class="detail">
<span class="info-label" translate>Reboot Suggested</span>
<span class="info-value">{{ erratum.reboot_suggested }}</span>
</div>

<div class="detail">
<span class="info-label" translate>Description</span>
<span class="info-value">
<p ng-repeat="paragraph in erratum.description.split('\n\n')">
{{ paragraph }}
</p>
</span>
</div>

<div class="detail">
<span class="info-label" translate>Solution</span>
<span class="info-value">
<p ng-repeat="paragraph in erratum.solution.split('\n\n')">
{{ paragraph }}
</p>
</span>
</div>

<div class="detail">
<span class="info-label" translate>Packages</span>
<span class="info-value">
<ul>
<li ng-repeat="package in erratum.packages">
{{ package }}
</li>
</ul>
</span>
</div>

</section>
</div>
</div>

<div name="errataHostsDetails" ng-show="showHosts">
<a ng-click="showHosts = false">
<i class="fa fa-double-angle-left"></i>
{{ "Back To Errata List" | translate }}
</a>

<h3 translate>Applicable hosts for {{ erratum.errata_id }} </h3>

<div class="details details-full">
<section>
<table class="table">
<thead>
<tr>
<th translate>
Name
</th>
</tr>
</thead>

<tbody>
<tr ng-repeat="host in erratum.applicable_hosts">
<td>{{ host.name }}</td>
</tr>
</tbody>
</table>
</section>
</div>
</div>

</div>

<div data-block="modal-footer">
Expand Down
4 changes: 2 additions & 2 deletions ...s/bastion_katello/content-hosts/bulk/views/content-hosts-bulk-host-collections-modal.html
View file
Open in desktop
Expand Up @@ -66,12 +66,12 @@ <h4 data-block="modal-header">Host Collection Membership Management</h4>
<tbody>
<tr bst-table-row ng-repeat="hostCollection in table.rows" row-select="hostCollection">
<td bst-table-cell>
<a ui-sref="host-collection.info({hostCollectionId: hostCollection.id})">
<a ng-click="ok(); " ui-sref="host-collection.info({hostCollectionId: hostCollection.id})" >
{{ hostCollection.name }}
</a>
</td>
<td bst-table-cell>
<a ui-sref="host-collection.content-hosts({hostCollectionId: hostCollection.id})">
<a ng-click="ok();" ui-sref="host-collection.hosts.list({hostCollectionId: hostCollection.id})">
{{ hostCollection.total_hosts }}
</a>
</td>
Expand Down
2 changes: 1 addition & 1 deletion ...ipts/bastion_katello/content-hosts/bulk/views/content-hosts-bulk-subscriptions-modal.html
View file
Open in desktop
Expand Up @@ -80,7 +80,7 @@ <h4 data-block="modal-header" translate>
<tr bst-table-row ng-repeat-start="(name, subscriptions) in groupedSubscriptions">
<td class="row-select"></td>
<td colspan="8">
<a ui-sref="subscription.info({subscriptionId: subscription.id})" class="confined-text">
<a ng-click="ok();" ui-sref="subscription.info({subscriptionId: subscription.id})" class="confined-text">
Copy link
Member

Choose a reason for hiding this comment

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

This confirmation modal doesn't lock out the yes/no buttons after the first click, so I can easily get 500 errors from the server by clicking yes/no multiple times before I'm redirected to the task progress.

Copy link
Member

Choose a reason for hiding this comment

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

This probably should be made into a separate issue rather than being fixed in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will go ahead open an issue around the same.

The transitionTo() method was used to redirect to the errata details and content hosts page specific to the errata action on host collections.
I am trying not to use those page as redirect links but content inside the existing modal seems more user friendly instead of the redirection.

{{ name }}
</a>
</td>
Expand Down
28 changes: 0 additions & 28 deletions ...app/assets/javascripts/bastion_katello/content-hosts/bulk/views/errata-content-hosts.html
View file
Open in desktop

This file was deleted.

59 changes: 0 additions & 59 deletions ...tello/app/assets/javascripts/bastion_katello/content-hosts/bulk/views/errata-details.html
View file
Open in desktop

This file was deleted.