Skip to content

Commit

Permalink
[SDESK-6504] fix: Ignore unlock ws notification if already unlocked (#…
Browse files Browse the repository at this point in the history
…1711)

* [SDESK-6504] fix: Ignore unlock ws notification if already unlocked (#1694)

* [SDESK-6504] fix: Ignore unlock ws notification if already unlocked
This was causing the Editor to act as if the Event/Planning item was not locked for editing

* Iterate pagination when getting item locks

* Fallback to getting lock directly from item

* Fix unit tests

* Include killed Events when getting locks

* fix(e2e): After daylight savings change
  • Loading branch information
MarkLark86 authored Aug 26, 2022
1 parent 0f76909 commit d81c068
Show file tree
Hide file tree
Showing 15 changed files with 159 additions and 66 deletions.
11 changes: 9 additions & 2 deletions client/actions/events/notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import eventsUi from './ui';
import main from '../main';
import planningApi from '../planning/api';
import {get} from 'lodash';
import {gettext, dispatchUtils, getErrorMessage} from '../../utils';
import {gettext, dispatchUtils, getErrorMessage, lockUtils} from '../../utils';
import eventsPlanning from '../eventsPlanning';

/**
Expand All @@ -30,8 +30,15 @@ const onEventCreated = (_e, data) => (
const onEventUnlocked = (_e, data) => (
(dispatch, getState) => {
if (data && data.item) {
const events = selectors.events.storedEvents(getState());
const state = getState();
const events = selectors.events.storedEvents(state);
let eventInStore = get(events, data.item, {});
const isCurrentlyLocked = lockUtils.isItemLocked(eventInStore, selectors.locks.getLockedItems(state));

if (!isCurrentlyLocked && eventInStore?.lock_session == null) {
// No need to announce an unlock, as we have already done so
return Promise.resolve(eventInStore);
}

dispatch(main.onItemUnlocked(data, eventInStore, ITEM_TYPE.EVENT));

Expand Down
2 changes: 2 additions & 0 deletions client/actions/events/tests/notifications_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,7 @@ describe('actions.events.notifications', () => {
beforeEach(() => {
store.initialState.events.events.e1.lock_user = 'ident1';
store.initialState.events.events.e1.lock_session = 'session1';
store.initialState.events.events.e1.lock_time = '2022-06-15T13:01:11+0000';
sinon.stub(main, 'changeEditorAction').callsFake(() => Promise.resolve());
});

Expand Down Expand Up @@ -577,6 +578,7 @@ describe('actions.events.notifications', () => {
{
item: 'e1',
user: 'ident2',
lock_session: 'session1',
etag: 'e123',
}
))
Expand Down
11 changes: 9 additions & 2 deletions client/actions/planning/notifications.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {get} from 'lodash';
import planning from './index';
import assignments from '../assignments/index';
import {gettext} from '../../utils';
import {gettext, lockUtils} from '../../utils';
import * as selectors from '../../selectors';
import {events, fetchAgendas} from '../index';
import main from '../main';
Expand Down Expand Up @@ -136,7 +136,14 @@ const onPlanningLocked = (e, data) => (
const onPlanningUnlocked = (_e, data) => (
(dispatch, getState) => {
if (get(data, 'item')) {
let planningItem = selectors.planning.storedPlannings(getState())[data.item];
const state = getState();
let planningItem = selectors.planning.storedPlannings(state)[data.item];
const isCurrentlyLocked = lockUtils.isItemLocked(planningItem, selectors.locks.getLockedItems(state));

if (!isCurrentlyLocked && planningItem?.lock_session == null) {
// No need to announce an unlock, as we have already done so
return Promise.resolve();
}

dispatch(main.onItemUnlocked(data, planningItem, ITEM_TYPE.PLANNING));

Expand Down
2 changes: 2 additions & 0 deletions client/actions/planning/tests/notifications_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ describe('actions.planning.notifications', () => {
store.initialState.planning.currentPlanningId = 'p1';
store.initialState.planning.plannings.p1.lock_user = 'ident1';
store.initialState.planning.plannings.p1.lock_session = 'session1';
store.initialState.planning.plannings.p1.lock_time = '2022-06-15T13:01:11+0000';
sinon.stub(main, 'changeEditorAction').callsFake(() => Promise.resolve());
});

Expand Down Expand Up @@ -301,6 +302,7 @@ describe('actions.planning.notifications', () => {
{
item: 'p1',
user: 'ident2',
lock_session: 'session1',
etag: 'e123',
}))
.then(() => {
Expand Down
9 changes: 3 additions & 6 deletions client/api/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,15 +110,12 @@ export function getEventByIds(
}

export function getLockedEvents(): Promise<Array<IEventItem>> {
return searchEvents({
return searchEventsGetAll({
lock_state: LOCK_STATE.LOCKED,
directly_locked: true,
only_future: false,
})
.then(modifyResponseForClient)
.then((response) => (
response._items
));
include_killed: true,
});
}

function getEventEditorProfile() {
Expand Down
7 changes: 3 additions & 4 deletions client/api/planning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,13 +125,12 @@ export function getPlanningByIds(
}

export function getLockedPlanningItems(): Promise<Array<IPlanningItem>> {
return searchPlanning({
return searchPlanningGetAll({
lock_state: LOCK_STATE.LOCKED,
directly_locked: true,
only_future: false,
})
.then(modifyResponseForClient)
.then((response) => response._items);
include_killed: true,
});
}

export function getLockedFeaturedPlanning(): Promise<Array<IFeaturedPlanningLock>> {
Expand Down
2 changes: 1 addition & 1 deletion client/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ export interface IAssignmentItem extends IBaseRestApiResponse {
version_creator: string;
firstcreated: string;
versioncreated: string;
type: string;
type: 'assignment';
lock_user: string;
lock_time: string | Date | moment.Moment;
lock_session: string;
Expand Down
19 changes: 10 additions & 9 deletions client/utils/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {get, map, isNil, sortBy, cloneDeep, omitBy, find, isEqual, pickBy, flatt
import {IMenuItem} from 'superdesk-ui-framework/react/components/Menu';

import {appConfig} from 'appConfig';
import {IEventItem} from '../interfaces';
import {IEventItem, ISession, ILockedItems} from '../interfaces';

import {
PRIVILEGES,
Expand Down Expand Up @@ -67,15 +67,16 @@ const isEventSameDay = (startingDate, endingDate) => (

const eventHasPlanning = (event) => get(event, 'planning_ids', []).length > 0;

const isEventLocked = (event, locks) =>
!isNil(event) && locks && (
event._id in locks.event ||
get(event, 'recurrence_id') in locks.recurring
);
function isEventLocked(event: IEventItem, locks: ILockedItems): boolean {
return lockUtils.getLock(event, locks) != null;
}

const isEventLockRestricted = (event, session, locks) =>
isEventLocked(event, locks) &&
!lockUtils.isItemLockedInThisSession(event, session, locks);
function isEventLockRestricted(event: IEventItem, session: ISession, locks: ILockedItems): boolean {
return (
isEventLocked(event, locks) &&
!lockUtils.isItemLockedInThisSession(event, session, locks)
);
}

const isEventCompleted = (event) => (get(event, 'completed'));

Expand Down
91 changes: 60 additions & 31 deletions client/utils/locks.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
import {isNil, get} from 'lodash';

import {
IEventItem,
IEventOrPlanningItem,
ILock,
ILockedItems,
IPlanningItem,
IAssignmentItem,
} from '../interfaces';
import {ITEM_TYPE} from '../constants';
import {getItemType, eventUtils, planningUtils, assignmentUtils, getItemId} from './index';
import {getItemType, eventUtils, planningUtils, assignmentUtils, timeUtils} from './index';

const isLockedByUser = (item, userId, action) => (
!isNil(get(item, 'lock_session')) &&
Expand All @@ -16,54 +25,74 @@ const getLockedUser = (item, lockedItems, users) => {
null;
};

const getLock = (item, lockedItems) => {
const itemId = getItemId(item);

if (!itemId) {
function getLock(item: IEventOrPlanningItem | IAssignmentItem | null, lockedItems: ILockedItems): ILock | null {
if (item?._id == null) {
return null;
}

switch (getItemType(item)) {
case ITEM_TYPE.EVENT:
} else if (item.type === 'event') {
return self.getEventLock(item, lockedItems);
case ITEM_TYPE.PLANNING:
} else if (item.type === 'planning') {
return self.getPlanningLock(item, lockedItems);
default:
if (get(lockedItems.assignment, itemId)) {
return lockedItems.assignment[itemId];
} else if (item.type === 'assignment') {
if (lockedItems.assignment[item._id] != null) {
return lockedItems.assignment[item._id];
} else if (item.lock_session != null) {
return {
action: item.lock_action,
item_id: item._id,
item_type: item.type,
session: item.lock_session,
time: item.lock_time == null ? undefined : timeUtils.getDateAsString(item.lock_time),
user: item.lock_user,
};
}

break;
}

return null;
};

const getEventLock = (item, lockedItems) => {
const itemId = getItemId(item);
}

if (get(lockedItems.recurring, get(item, 'recurrence_id'))) {
function getEventLock(item: IEventItem | null, lockedItems: ILockedItems): ILock | null {
if (item?._id == null) {
return null;
} else if (item.recurrence_id != null && lockedItems.recurring[item.recurrence_id] != null) {
return lockedItems.recurring[item.recurrence_id];
} else if (get(lockedItems.event, itemId)) {
return lockedItems.event[itemId];
} else if (lockedItems.event[item._id] != null) {
return lockedItems.event[item._id];
} else if (item.lock_session != null) {
return {
action: item.lock_action,
item_id: item._id,
item_type: item.type,
session: item.lock_session,
time: item.lock_time == null ? undefined : timeUtils.getDateAsString(item.lock_time),
user: item.lock_user,
};
}

return null;
};
}

const getPlanningLock = (item, lockedItems) => {
const itemId = getItemId(item);

if (get(lockedItems.planning, itemId)) {
return lockedItems.planning[itemId];
} else if (get(lockedItems.recurring, get(item, 'recurrence_id'))) {
function getPlanningLock(item: IPlanningItem | null, lockedItems: ILockedItems): ILock | null {
if (item?._id == null) {
return null;
} else if (lockedItems.planning[item._id] != null) {
return lockedItems.planning[item._id];
} else if (item.recurrence_id != null && lockedItems.recurring[item.recurrence_id] != null) {
return lockedItems.recurring[item.recurrence_id];
} else if (get(lockedItems.event, get(item, 'event_item'))) {
} else if (item.event_item != null && lockedItems.event[item.event_item] != null) {
return lockedItems.event[item.event_item];
} else if (item.lock_session != null) {
return {
action: item.lock_action,
item_id: item._id,
item_type: item.type,
session: item.lock_session,
time: item.lock_time == null ? undefined : timeUtils.getDateAsString(item.lock_time),
user: item.lock_user,
};
}

return null;
};
}

const getLockAction = (item, lockedItems) => (
get(self.getLock(item, lockedItems), 'action')
Expand Down
20 changes: 11 additions & 9 deletions client/utils/planning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import {
IPlanningCoverageItem,
IPlanningNewsCoverageStatus,
IG2ContentType,
ISession,
ILockedItems,
} from '../interfaces';
import {stripHtmlRaw} from 'superdesk-core/scripts/apps/authoring/authoring/helpers';

Expand Down Expand Up @@ -202,16 +204,16 @@ const canAddCoverages = (planning, event, privileges, session, locks) => (
!isItemExpired(planning)
);

const isPlanningLocked = (plan, locks) =>
!isNil(plan) && (
plan._id in locks.planning ||
get(plan, 'event_item') in locks.event ||
get(plan, 'recurrence_id') in locks.recurring
);
function isPlanningLocked(plan: IPlanningItem, locks: ILockedItems): boolean {
return lockUtils.getLock(plan, locks) != null;
}

const isPlanningLockRestricted = (plan, session, locks) =>
isPlanningLocked(plan, locks) &&
!lockUtils.isItemLockedInThisSession(plan, session, locks);
function isPlanningLockRestricted(plan: IPlanningItem, session: ISession, locks: ILockedItems): boolean {
return (
isPlanningLocked(plan, locks) &&
!lockUtils.isItemLockedInThisSession(plan, session, locks)
);
}

/**
* Get the array of coverage content type and color base on the scheduled date
Expand Down
1 change: 1 addition & 0 deletions client/utils/testApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Object.assign(superdeskApi, {
},
utilities: {
querySelectorParent: querySelectorParent,
dateToServerString: (date) => date.toISOString().slice(0, 19) + '+0000',
},
privileges: {
hasPrivilege: (privilege: string) => privileges[privilege] === 1
Expand Down
Loading

0 comments on commit d81c068

Please sign in to comment.