Skip to content
This repository has been archived by the owner on May 3, 2022. It is now read-only.

EZP-27914: As an Editor I want to remove permanently a language from a content item #949

Merged
merged 2 commits into from
Feb 19, 2018

Conversation

adamwojs
Copy link
Member

@adamwojs adamwojs commented Feb 8, 2018

JIRA: https://jira.ez.no/browse/EZP-27914
Depends on ezsystems/ez-js-rest-client#86

Description

Impl. of EZP-27914: As an Editor I want to remove permanently a language from a content item

screenshot-2018-2-8 carly joyner ez platform - ez platform ui 1

Note:

if ( !this.get('id') ) {
return callback(false);
}
contentService.removeTranslation(this.get('id'), languageCode, callback);
Copy link
Member

Choose a reason for hiding this comment

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

an empty line for readability

@@ -14,7 +14,11 @@ YUI.add('ez-translateactionview', function (Y) {
var events = {
'.ez-newtranslation-button': {
'tap': '_newTranslationLanguageSelectionBox',
}
},

Copy link
Member

Choose a reason for hiding this comment

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

not needed empty line

};

e.preventDefault();
this.fire('deleteTranslation', data);
Copy link
Member

Choose a reason for hiding this comment

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

missing doc

@@ -567,6 +568,60 @@ YUI.add('ez-locationviewviewservice', function (Y) {
this.set('languageCode', this.get('request').params.languageCode);
},

/**
* Removes translation form content.
Copy link
Member

Choose a reason for hiding this comment

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

I think it should be ...from content.

};

content.removeTranslation(options, languageCode, Y.bind(function() {
this._notify(
Copy link
Member

Choose a reason for hiding this comment

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

I don't see any error handling for remove translation or I missed that. This assumes it will be always the success.

},

/**
* `deleteTranslationAction` event handler,
Copy link
Member

Choose a reason for hiding this comment

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

deleteTranslation this is the name of the event.

'content-delete-translation-' + content.get('id'), 'error', 0
);
} else {
this.fire('confirmBoxOpen', {
Copy link
Member

Choose a reason for hiding this comment

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

doc


content.removeTranslation(options, languageCode, Y.bind(function() {
this._notify(
Y.eZ.trans('success.delete.content.translation', {}, 'bar'),
Copy link
Member

Choose a reason for hiding this comment

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

In translation there is a placeholder for language I don't see it here.

@adamwojs
Copy link
Member Author

adamwojs commented Feb 9, 2018

PR updated according to @dew326 suggestions 😉


linkUtils.remove(this.state.element, {advance: true});

selection.selectBookmarks(bookmarks);

this.props.cancelExclusive();

if (navigator.userAgent.indexOf('Chrome') > -1) {
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like I've deleted it by accident. Thanks for vigilance

* Fired when the translation is deleted
*
* @event deleteTranslation
* @param {e.translation} Language code of translation
Copy link
Member

Choose a reason for hiding this comment

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

data.translation

Y.eZ.trans('failed.delete.content.translation', { language: languageCode }, 'bar'),
'content-delete-translation-' + content.get('id'), 'error', 0
);
return ;
Copy link
Member

Choose a reason for hiding this comment

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

not needed space

/**
* Opens confirmation modal of deleting translation
*
* @event deleteTranslation
Copy link
Member

Choose a reason for hiding this comment

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

confirmBoxOpen

contentService = capi.getContentService();

if ( !this.get('id') ) {
return callback(false);
Copy link
Member

Choose a reason for hiding this comment

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

This will be displayed as a success, I'm not sure if we want this.

Copy link
Member Author

Choose a reason for hiding this comment

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

True. There is no checking of id in other methods (besides delete) so I've removed this condition (API should return 404 in this case).

@adamwojs adamwojs force-pushed the ezp_27914 branch 2 times, most recently from feb1daf to 197807b Compare February 9, 2018 11:50
@andrerom
Copy link
Contributor

@adamwojs No rush on this one, but tests are failing here while they are passing on 1.13 now.

@andrerom andrerom merged commit 8792449 into ezsystems:1.13 Feb 19, 2018
@andrerom
Copy link
Contributor

Got geen light from @SylvainGuittard on this on friday, so adding this to 1.13.1-rc2 for testing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

3 participants