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

[RFR] Delete view: back to previous state, not edit or list view #538

Merged
merged 3 commits into from
Jul 24, 2015
Merged

[RFR] Delete view: back to previous state, not edit or list view #538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9acf616
Delete view: back to previous state, not edit or list view
Jun 30, 2015
ba10f2a
Fixes from fzaninotto's comment, adjusted failing e2e test
Jul 1, 2015
f185942
Fixes from jpetitcolas's comments
Jul 2, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 7 additions & 16 deletions src/javascripts/ng-admin/Crud/delete/DeleteController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
define(function () {
'use strict';

var DeleteController = function ($scope, $state, WriteQueries, notification, params, view, entry) {
var DeleteController = function ($scope, $window, WriteQueries, notification, params, view, entry) {
this.$scope = $scope;
this.$state = $state;
this.$window = $window;
this.WriteQueries = WriteQueries;
this.entityLabel = params.entity;
this.entityId = params.id;
Expand All @@ -22,14 +22,11 @@ define(function () {

DeleteController.prototype.deleteOne = function () {
var notification = this.notification,
$state = this.$state, entityName = this.entity.name();
entityName = this.entity.name(),
$window = this.$window;

this.WriteQueries.deleteOne(this.view, this.entityId).then(function () {

$state.go($state.get('list'), angular.extend({
entity: entityName,
id: this.entityId
}, $state.params));
this.back();
Copy link

Choose a reason for hiding this comment

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

looks to me like this causes a regression: if you click "delete" while on the showView page, the new code sends you "back" to that page which 404's.

Copy link
Member

Choose a reason for hiding this comment

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

@ahgittin this is taken care of in #642. Could you test it and report your findings in the PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

@ahgittin: incoming #642 indeed fixes this issue. If you are on the show view and click delete, the controller is going to check if the previous page is related to the deleted entity. If so, it redirects you to the list view. Otherwise, to the previous page.

notification.log('Element successfully deleted.', { addnCls: 'humane-flatty-success' });
}.bind(this), function (response) {
// @TODO: share this method when splitting controllers
Expand All @@ -43,23 +40,17 @@ define(function () {
};

DeleteController.prototype.back = function () {
Copy link
Member

Choose a reason for hiding this comment

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

Why not implement the "back" logic in this method instead?

var $state = this.$state;

$state.go($state.get('edit'), angular.extend({
entity: this.entity.name(),
id: this.entityId
}, $state.params));
this.$window.history.back();
};

DeleteController.prototype.destroy = function () {
this.$scope = undefined;
this.WriteQueries = undefined;
this.$state = undefined;
this.view = undefined;
this.entity = undefined;
};

DeleteController.$inject = ['$scope', '$state', 'WriteQueries', 'notification', 'params', 'view', 'entry'];
DeleteController.$inject = ['$scope', '$window', 'WriteQueries', 'notification', 'params', 'view', 'entry'];

return DeleteController;
});
5 changes: 0 additions & 5 deletions src/javascripts/test/e2e/ListViewSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,6 @@ describe('ListView', function () {
return $$('button.btn-default');
}).then(function (elements) {
return elements[0].click();
}).then(function() {
expect(browser.getCurrentUrl()).toBe(browser.baseUrl + '/#/comments/edit/2');
return $$('ma-list-button a');
}).then(function (elements) {
return elements[0].click();
}).then(function() {
expect(browser.getCurrentUrl()).toBe(listUrl);
});
Expand Down