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

Allow to restore assets after being deleted ( datasets, organizations and reuses ). #1901

Merged

Conversation

micael-grilo
Copy link
Contributor

Context:
When I delete an asset ( dataset, organization, reuse), I want to be able to restore it.

Resume:
Assets can be accidentally deleted, and there's no way to easily restore them, neither through API or through Admin area. This PR adds:

  • Ability to restore a asset through API:
    -- Sending a POST with the deleted field Empty ("deleted:"null") will restore the asset.

  • Change Admin Area to accommodate this new feature.
    -- In admin area you can now easily restore an asset that was deleted.
    image

Copy link
Contributor

@noirbizarre noirbizarre left a comment

Choose a reason for hiding this comment

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

Interesting feature and way to implement it.
Just fix the review comments and remove or split out the badge part.
Thanks 👌
NB: Don't forget to provide a CHANGELOG entry and to rebase your branch on the current master

},
methods: {
confirm() {
var restore_dataset = this.dataset;
Copy link
Contributor

Choose a reason for hiding this comment

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

Use const instead of var because we are trying to use ES6 as much as possible

},
methods: {
confirm() {
var restore_organization = this.organization;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

},
methods: {
confirm() {
var restore_reuse = this.reuse;
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here

@@ -203,6 +213,12 @@ export default {
{dataset: this.dataset}
);
},
confirm_restore() {
var m = this.$root.$modal(
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to store the reference m, it's not used later.

@@ -258,6 +274,8 @@ export default {
class: 'danger',
label: this._('Deleted')
}];
} else {
this.badges = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is related to this pull-request. Remove it or submit another one for this case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image
The purpose of this line is to clear the "Deleted" red badge.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK I understand 👍
But on some datasets/reuses/organization, there is more than one badge (more than only the deleted badge), you may need to do some filtering to only remove the deleted badge

@@ -189,6 +205,8 @@ export default {
class: 'danger',
label: this._('Deleted')
}];
} else {
this.badges = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Same, not related

API.organizations.update_organization(
{org: this.organization.id, payload: restore_organization},
(response) => {
this.organization.fetch();
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid an extra request, you can directly use the response payload to update the organization: this.organization.on_fetched(reponse)

restore_dataset.deleted = null;
API.datasets.update_dataset({dataset: this.dataset.id, payload: restore_dataset},
(response) => {
this.dataset.fetch();
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid an extra request, you can directly use the response payload to update the dataset: this.dataset.on_fetched(reponse)

restore_reuse.deleted = null;
API.reuses.update_reuse({reuse: this.reuse.id, payload: restore_reuse},
(response) => {
this.reuse.fetch();
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid an extra request, you can directly use the response payload to update the reuse: this.reuse.on_fetched(reponse)

@noirbizarre noirbizarre modified the milestones: 1.6.0, 1.6.1 Oct 4, 2018
@noirbizarre noirbizarre merged commit 60a2e33 into opendatateam:master Oct 4, 2018
@micael-grilo micael-grilo deleted the feature/allow_assets_restore branch October 17, 2018 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants