Fix qa reported bugs #2216
Fix qa reported bugs #2216
Conversation
|
TODO:
Note: Look into |
| ): Promise<void> => { | ||
| for (const exposed of getEmbeddedItemsExposed(editorType, itemType)) { | ||
| ): Promise<Array<T>> => { | ||
| const updatedItems: Array<T> = []; |
There was a problem hiding this comment.
what's the purpose of adding this?
There was a problem hiding this comment.
I use the result of handleEmbeddedItems in ItemManager.tsx to update related_events.
There was a problem hiding this comment.
Could you respond to everything in one review?
| } | ||
| } | ||
|
|
||
| if (updates.type === 'planning' && (updates.related_events ?? []).length > 0) { |
There was a problem hiding this comment.
What are you doing here that wasn't working with the previous version of code?
There was a problem hiding this comment.
I'm updating related_events
client/components/editor-standalone/field-definitions/calendars.ts
Outdated
Show resolved
Hide resolved
client/components/editor-standalone/field-definitions/all-day.ts
Outdated
Show resolved
Hide resolved
| label: getVocabularyItemFieldTranslated( | ||
| option, | ||
| 'label', | ||
| 'en', |
There was a problem hiding this comment.
language shouldn't be hardcoded
| label: getVocabularyItemFieldTranslated( | ||
| option, | ||
| 'label', | ||
| 'en', |
There was a problem hiding this comment.
language shouldn't be hardcoded
client/components/editor-standalone/field-definitions/lanugage.ts
Outdated
Show resolved
Hide resolved
| ]} | ||
| > | ||
| {(res) => { | ||
| const files: Array<IFile> = res[0]._items; | ||
| const files: Array<IFile> = res?.[0]?._items ?? []; |
There was a problem hiding this comment.
after superdesk/superdesk-client-core#4762 you can revert this line to how it was
| // otherwise trying to save the same item twice would happen | ||
| if (isTemporaryId(original._id) && !isTemporaryId(updates._id)) { | ||
| editor.form.changeField( | ||
| '_unsaved_related_events', |
| profile: 'event' | 'planning', | ||
| item: T, | ||
| onSave: (current: T, original: T) => Promise<T>, | ||
| ): IAuthoringStorage<T> { | ||
| /** | ||
| * Timeout for 500 seconds to let newly added embedded events finish autosaving. |
There was a problem hiding this comment.
Timing can't be relied upon. We should do a better solution. Could you elaborate what issue you're trying to address here?
Why did you drop my usage of spread operator to handle validation?
| return new Promise<T>((resolve) => { | ||
| setTimeout(() => { | ||
| autosave | ||
| .getById(profile === 'event' ? 'event' : 'planning', id) |
There was a problem hiding this comment.
profile === 'event' ? 'event' : 'planning'
It's a trap to do it like that. If more items are added to the union type the code will fail silently. And we want it to fail loud and clear :D Use if/else if/assert never
| @@ -750,6 +750,7 @@ export interface IPlanningItem extends IBaseRestApiResponse { | |||
| // added by client - should be dropped before sending to server | |||
| event?: IEventItem; | |||
|
|
|||
| _unsaved_related_events?: Array<IPlanningRelatedEventLink>; | |||
There was a problem hiding this comment.
add a comment on how it is mean to be used. sent to autosave, but not save?
| @@ -218,6 +218,7 @@ | |||
| "mapping": not_analyzed, | |||
| "nullable": True, | |||
| }, | |||
| "_unsaved_related_events": {"type": "list", "nullable": True}, | |||
There was a problem hiding this comment.
can this be added to autosave only?
| const clonedValue = cloneDeep(item.dates.recurring_rule); | ||
|
|
||
| if (clonedValue?.until != null) { | ||
| set(clonedValue, nameof<typeof clonedValue>('until'), moment(clonedValue.until)); |
There was a problem hiding this comment.
since value update path is static rather than dynamic - no need to use lodash.set here. It'd be cleaner to use the usual immutable updating via spreading.
STT-31
Front-end checklist
memoorPureComponentto define new React components (and updates existing usages in modified code segments)lodash.getwith optional chaining and nullish coalescing for modified code segmentssuperdeskApi)superdesk-ui-frameworkandsuperdeskApiwhen possible instead of using ones defined in this repository.planningApiwhere it is possible to usesuperdeskApiplanningApi)