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

fix media metadata editor changes not being saved #4572

Merged
merged 11 commits into from
Jul 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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',
tomaskikutis marked this conversation as resolved.
Show resolved Hide resolved
'@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');
});
15 changes: 13 additions & 2 deletions e2e/client/playwright/page-object-models/authoring.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import {Page} from '@playwright/test';
import {Locator, Page} from '@playwright/test';
import {s} from '../utils';

export class Authoring {
private page: Page;
protected page: Page;

constructor(page: Page) {
this.page = page;
Expand All @@ -21,4 +21,15 @@ export class Authoring {
.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 {
tomaskikutis marked this conversation as resolved.
Show resolved Hide resolved
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();
Copy link
Member

Choose a reason for hiding this comment

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

is change-image image selector still inside "media-editor"? If it is, it'd be good to scope it:

await this.page.locator(s('media-editor', 'change-image', 'done')).click();

The reason being not uniqueness, but documentation - ability to quickly understand the hierarchy without having to look it up inside dev tools.

Copy link
Member Author

Choose a reason for hiding this comment

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

those are on the same level

}
}
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 @@ -60,4 +60,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,
},
}
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

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.eventListenersToRemoveOnUnmount = [];
$scope.toDeskEnabled = false; // Send an Item to a desk
Expand Down Expand Up @@ -187,7 +184,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 @@ -270,11 +267,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 @@ -557,9 +554,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 @@ -676,9 +671,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 @@ -859,7 +852,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 @@ -882,6 +875,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
Loading