Skip to content

Commit

Permalink
fix media metadata editor changes not being saved (superdesk#4572)
Browse files Browse the repository at this point in the history
* fix media metadata editor changes not being saved

it was calling authoring editor instances to generate
html which then overwrote any changes done in modal.

SDESK-7304

* trigger actions

* add test from authoring to modal

* trigger autosave when opening modal

* only trigger autosave if dirty

* fix protractor

* fix failing protractor tests

* handle feedback

* docs

* create picture authoring object

* tweak test
  • Loading branch information
petrjasek committed Jul 22, 2024
1 parent c4afcc0 commit fb11f62
Show file tree
Hide file tree
Showing 16 changed files with 192 additions and 37 deletions.
2 changes: 2 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ module.exports = Object.assign({}, sharedConfigs, {
'@typescript-eslint/ban-types': 'off',
'@typescript-eslint/no-inferrable-types': 'off',
'@typescript-eslint/no-this-alias': 'off',
'@typescript-eslint/no-explicit-any': 'off',
'@typescript-eslint/no-unused-vars': 'off',
}),
plugins: [...(sharedConfigs.plugins ?? []), '@typescript-eslint'],

Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -90,5 +90,5 @@ jobs:

- name: Server Logs
if: ${{ failure() }}
run: docker compose logs superdesk
run: docker compose logs server
working-directory: e2e/server
51 changes: 51 additions & 0 deletions e2e/client/playwright/authoring.picture.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import {test, expect} from '@playwright/test';
import {Monitoring} from './page-object-models/monitoring';
import {restoreDatabaseSnapshot} from './utils';
import {MediaEditor} from './page-object-models/media-editor';
import {PictureAuthoring} from './page-object-models/authoring';
import {MediaUpload} from './page-object-models/upload';

test.setTimeout(30000);

/**
* upload a picture
* edit metadata
* test metadata changes from modal are visible in the editor
*/
test('media metadata editor', async ({page}) => {
await restoreDatabaseSnapshot();

const upload = new MediaUpload(page);
const monitoring = new Monitoring(page);
const mediaEditor = new MediaEditor(page);
const pictureAuthoring = new PictureAuthoring(page);

await page.goto('/#/workspace/monitoring');

await monitoring.selectDeskOrWorkspace('Sports');
await monitoring.openMediaUploadView();

await upload.selectFile('iptc-photo.jpg');

await expect(mediaEditor.field('field--headline')).toContainText('The Headline');

await mediaEditor.field('field--headline').clear();
await mediaEditor.field('field--headline').fill('picture');

await upload.startUpload();

await monitoring.executeActionOnMonitoringItem(monitoring.getArticleLocator('picture'), 'Edit');

await pictureAuthoring.openMetadataEditor();

await mediaEditor.field('field--description_text').fill('test description');
await mediaEditor.saveMetadata();

await expect(pictureAuthoring.field('field--description_text')).toContainText('test description');

await pictureAuthoring.field('field--description_text').fill('new description');

await pictureAuthoring.openMetadataEditor();

await expect(mediaEditor.field('field--description_text')).toContainText('new description');
});
35 changes: 35 additions & 0 deletions e2e/client/playwright/page-object-models/authoring.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import {Locator, Page} from '@playwright/test';
import {s} from '../utils';

export class Authoring {
protected page: Page;

constructor(page: Page) {
this.page = page;
}

async executeActionInEditor(...actionPath: Array<string>): Promise<void> {
await this.page.locator(s('authoring-topbar', 'actions-button')).click();

const actionsWithoutLast = actionPath.slice(0, actionPath.length - 1);

for (const action of actionsWithoutLast) {
await this.page.locator(s('actions-list')).getByRole('button', {name: action}).hover();
}

await this.page.locator(s('actions-list'))
.getByRole('button', {name: actionPath[actionPath.length - 1]})
.click();
}

field(field: string): Locator {
return this.page.locator(s('authoring', field)).getByRole('textbox');
}
}

export class PictureAuthoring extends Authoring {
async openMetadataEditor(): Promise<void> {
await this.page.locator(s('authoring-field=media', 'image-overlay')).hover();
await this.page.locator(s('authoring-field=media', 'edit-metadata')).click();
}
}
19 changes: 19 additions & 0 deletions e2e/client/playwright/page-object-models/media-editor.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import {Locator, Page} from '@playwright/test';
import {s} from '../utils';

export class MediaEditor {
private page: Page;

constructor(page: Page) {
this.page = page;
}

field(field: string): Locator {
return this.page.locator(s('media-metadata-editor', field)).getByRole('textbox');
}

async saveMetadata(): Promise<void> {
await this.page.locator(s('media-editor', 'apply-metadata-button')).click();
await this.page.locator(s('change-image', 'done')).click();
}
}
9 changes: 9 additions & 0 deletions e2e/client/playwright/page-object-models/monitoring.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,4 +50,13 @@ export class Monitoring {
}
}
}

async openMediaUploadView(): Promise<void> {
await this.page.locator(s('content-create')).click();
await this.page.locator(s('content-create-dropdown')).getByRole('button', {name: 'Upload media'}).click();
}

getArticleLocator(headline: string): Locator {
return this.page.locator(s('article-item=' + headline));
}
}
21 changes: 21 additions & 0 deletions e2e/client/playwright/page-object-models/upload.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import {Page} from '@playwright/test';
import {s} from '../utils';
import path from 'path';

export class MediaUpload {
private page: Page;

constructor(page: Page) {
this.page = page;
}

async selectFile(filename: string): Promise<void> {
await this.page.locator(s('file-upload', 'select-file-button')).click();
await this.page.locator(s('file-upload', 'image-upload-input'))
.setInputFiles(path.join('test-files', filename));
}

async startUpload(): Promise<void> {
await this.page.locator(s('file-upload', 'multi-image-edit--start-upload')).click();
}
}
9 changes: 1 addition & 8 deletions e2e/client/specs/authoring_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,7 @@ function uploadMedia(imagePathAbsolute) {
el(['media-metadata-editor', 'field--headline'], by.tagName('[contenteditable]'))
.sendKeys('image headline');
el(['media-metadata-editor', 'field--slugline'], by.tagName('[contenteditable]'))
.sendKeys('image headline');
el(['media-metadata-editor', 'field--alt_text'], by.tagName('[contenteditable]'))
.sendKeys('image alt text');

selectFromMetaTermsDropdown('anpa_category', ['Finance']);

selectFromMetaTermsDropdown('subject', ['arts, culture and entertainment', 'archaeology']);

.sendKeys('image slugline');
el(['media-metadata-editor', 'field--description_text'], by.tagName('[contenteditable]'))
.sendKeys('image description');

Expand Down
Binary file added e2e/client/test-files/iptc-photo.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 3 additions & 2 deletions e2e/server/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
name: superdesk-client-core_e2e
version: "3.2"
services:
redis:
Expand All @@ -22,7 +23,7 @@ services:
command: --replSet rs0

elastic:
image: docker.elastic.co/elasticsearch/elasticsearch-oss:7.10.2
image: docker.elastic.co/elasticsearch/elasticsearch:7.17.22
ports:
- "9200:9200"
networks:
Expand All @@ -33,7 +34,7 @@ services:
tmpfs:
- /usr/share/elasticsearch/data

superdesk:
server:
build: .
ports:
- "5000:5000"
Expand Down
18 changes: 18 additions & 0 deletions e2e/server/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,21 @@
LEGAL_ARCHIVE = True

DEFAULT_TIMEZONE = "Europe/London"

VALIDATOR_MEDIA_METADATA = {
"slugline": {
"required": False,
},
"headline": {
"required": False,
},
"description_text": {
"required": True,
},
"byline": {
"required": False,
},
"copyrightnotice": {
"required": False,
},
}
7 changes: 4 additions & 3 deletions scripts/apps/archive/views/upload.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@
get-icon-for-item-type="getIconForItemType"
get-progress="getProgress"
on-remove-item="onRemoveItem"
upload-in-progress="saving">

upload-in-progress="saving",
data-test-id="file-upload"
>
<sd-multi-edit-select-desk>
<div ng-if="deskSelectionAllowed === true && desks !== null" class="dropdown dropdown--boxed" dropdown is-open="isOpen">
<button class="dropdown__toggle dropdown__toggle--small dropdown__toggle--hollow dropdown-toggle" dropdown__toggle aria-haspopup="true" aria-expanded="false">
Expand All @@ -35,7 +36,7 @@
<div class="upload__info-icon"></div>
<h3 class="upload__info-heading" translate>Drag Your Files Here</h3>
<div class="upload__info-label" translate>or</div>
<button ng-click="invokeImagesInput()" class="btn btn--hollow upload__info-button" translate>Select them from folder</button>
<button ng-click="invokeImagesInput()" data-test-id="select-file-button" class="btn btn--hollow upload__info-button" translate>Select them from folder</button>
<input ng-if="uniqueUpload" id="images-input" type="file" ngf-select="addFiles($files)" class="hide" data-test-id="image-upload-input">
<input ng-if="!uniqueUpload" id="images-input" type="file" ngf-select="addFiles($files)" multiple class="hide" data-test-id="image-upload-input">
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ interface IScope extends ng.IScope {
extra: any;
refreshTrigger: number;
autosave(item: any): any;
generateHtml(): void;
modifySignOff(item: any): void;
updateDateline(item: any, city: any): void;
resetNumberOfDays(dateline: any, datelineMonth?: any): void;
Expand Down Expand Up @@ -342,6 +343,10 @@ export function ArticleEditDirective(
* @description Opens the Change Image Controller to modify the image metadata.
*/
scope.editMedia = (defaultTab = 'view') => {
// generate html before opening the modal to make
// any changes done in the authoring visible there
scope.generateHtml();

let showTabs = [];

scope.mediaLoading = true;
Expand Down Expand Up @@ -388,7 +393,7 @@ export function ArticleEditDirective(
scope.articleEdit.$setDirty();
}
} else {
scope.save();
scope.save({generateHtml: false});
}
})
.finally(() => {
Expand Down
23 changes: 10 additions & 13 deletions scripts/apps/authoring/authoring/directives/AuthoringDirective.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,9 @@ export function AuthoringDirective(
$scope.tabsPinned = false;

var _closing;
var mediaFields = {};
var userDesks;

const UNIQUE_NAME_ERROR = gettext('Error: Unique Name is not unique.');
const MEDIA_TYPES = ['video', 'picture', 'audio'];
const isPersonalSpace = $location.path() === '/workspace/personal';

$scope.toDeskEnabled = false; // Send an Item to a desk
$scope.closeAndContinueEnabled = false; // Create an update of an item and Close the item.
Expand Down Expand Up @@ -186,7 +183,7 @@ export function AuthoringDirective(
function getCurrentTemplate() {
const item: IArticle | null = $scope.item;

if (item.type === 'composite') {
if (item.type !== 'text') {
$scope.currentTemplate = {};
} else {
if (typeof item?.template !== 'string') {
Expand Down Expand Up @@ -269,11 +266,11 @@ export function AuthoringDirective(
/**
* Create a new version
*/
$scope.save = function() {
$scope.save = function({generateHtml = true} = {}) {
return authoring.save(
$scope.origItem,
$scope.item,
$scope.requestEditor3DirectivesToGenerateHtml,
generateHtml ? $scope.requestEditor3DirectivesToGenerateHtml : [],
).then((res) => {
$scope.dirty = false;
_.merge($scope.item, res);
Expand Down Expand Up @@ -556,9 +553,7 @@ export function AuthoringDirective(
// delay required for loading state to render
// before possibly long operation (with huge articles)
setTimeout(() => {
for (const fn of $scope.requestEditor3DirectivesToGenerateHtml) {
fn();
}
$scope.generateHtml();

resolve();
});
Expand Down Expand Up @@ -675,9 +670,7 @@ export function AuthoringDirective(
_closing = true;

// Request to generate html before we pass scope variables
for (const fn of ($scope.requestEditor3DirectivesToGenerateHtml ?? [])) {
fn();
}
$scope.generateHtml();

// returned promise used by superdesk-fi
return authoringApiCommon.closeAuthoringStep2($scope, $rootScope);
Expand Down Expand Up @@ -858,7 +851,7 @@ export function AuthoringDirective(
$scope.firstLineConfig.wordCount = $scope.firstLineConfig.wordCount ?? true;

const _autosave = debounce((timeout) => {
$scope.requestEditor3DirectivesToGenerateHtml.forEach((fn) => fn());
$scope.generateHtml();

return authoring.autosave(
$scope.item,
Expand All @@ -881,6 +874,10 @@ export function AuthoringDirective(
_autosave(timeout);
};

$scope.generateHtml = () => {
$scope.requestEditor3DirectivesToGenerateHtml.forEach((fn) => fn());
};

$scope.sendToNextStage = function() {
sdApi.article.sendItemToNextStage($scope.item).then(() => {
$scope.$applyAsync();
Expand Down
17 changes: 10 additions & 7 deletions scripts/apps/authoring/views/article-edit.html
Original file line number Diff line number Diff line change
Expand Up @@ -279,18 +279,20 @@
sd-validation-error="error.media"
data-required="schema.media.required"
tabindex="{{editor.media.order}}"
sd-width="{{editor.media.sdWidth|| 'full'}}">

sd-width="{{editor.media.sdWidth|| 'full'}}"
data-test-id="authoring-field"
data-test-value="media"
>
<div ng-if="item.type == 'picture' || item.type == 'graphic'" class="full-preview" sd-ratio-calc>
<div class="media-item__item">
<div class="item-association item-association--preview">
<div class="item-association__image-container" sd-item-rendition data-item="item" data-rendition="baseImage"></div>

<div class="item-association__image-overlay" ng-if="_editable">
<div class="item-association__icons-block">
<a class="item-association__image-action" sd-tooltip="{{'Edit metadata' | translate}}" ng-click="editMedia('view')" aria-label="{{ 'Edit metadata' | translate}}"><i class="icon-pencil"></i></a>
<a class="item-association__image-action" sd-tooltip="{{'Edit image' | translate}}" ng-click="editMedia('image-edit')" aria-label="{{ 'Edit image' | translate}}"><i class="icon-switches"></i></a>
<a class="item-association__image-action" sd-tooltip="{{'Edit crops' | translate}}" ng-click="editMedia('crop')" ng-if="metadata.crop_sizes" aria-label="{{ 'Edit crops' | translate}}"><i class="icon-crop"></i></a>
<div class="item-association__image-overlay" ng-if="_editable" data-test-id="image-overlay">
<div class="item-association__icons-block" data-test-id="image-overlay-actions">
<a class="item-association__image-action" data-test-id="edit-metadata" sd-tooltip="{{'Edit metadata' | translate}}" ng-click="editMedia('view')" aria-label="{{ 'Edit metadata' | translate}}"><i class="icon-pencil"></i></a>
<a class="item-association__image-action" data-test-id="edit-image" sd-tooltip="{{'Edit image' | translate}}" ng-click="editMedia('image-edit')" aria-label="{{ 'Edit image' | translate}}"><i class="icon-switches"></i></a>
<a class="item-association__image-action" data-test-id="crop" sd-tooltip="{{'Edit crops' | translate}}" ng-click="editMedia('crop')" ng-if="metadata.crop_sizes" aria-label="{{ 'Edit crops' | translate}}"><i class="icon-crop"></i></a>
</div>
</div>
</div>
Expand Down Expand Up @@ -371,6 +373,7 @@
data-required="schema.description_text.required"
data-validation-error="error.description_text"
data-validate-characters="schema.description_text.validate_characters"
data-test-id="field--description_text"
></div>
</div>

Expand Down
Loading

0 comments on commit fb11f62

Please sign in to comment.