From 40a8d5685e3f5ea745ee9d6199e5e78e0ed32a34 Mon Sep 17 00:00:00 2001 From: jums Date: Mon, 19 Feb 2024 10:57:19 +0100 Subject: [PATCH] handle ordinal life cycle - with a temporary script to create ordinals on existing elements - allow to update ordinals, by accepting paginated elements subset for performance and client friendlyness - should test that removing element then reordering is ok --- ...assign_elements_ordinal_to_all_listings.ts | 89 +++++++++++++++++++ server/controllers/listings/lib/elements.ts | 26 ++++++ server/controllers/listings/lib/listings.ts | 24 +++-- server/controllers/listings/listings.ts | 2 + server/controllers/listings/reorder.ts | 30 +++++++ server/models/element.ts | 2 + tests/api/fixtures/listings.ts | 11 +++ tests/api/listings/ordinal.test.ts | 84 ++++++++++++++++- 8 files changed, 261 insertions(+), 7 deletions(-) create mode 100644 scripts/db_actions/assign_elements_ordinal_to_all_listings.ts create mode 100644 server/controllers/listings/reorder.ts diff --git a/scripts/db_actions/assign_elements_ordinal_to_all_listings.ts b/scripts/db_actions/assign_elements_ordinal_to_all_listings.ts new file mode 100644 index 000000000..c103f7f20 --- /dev/null +++ b/scripts/db_actions/assign_elements_ordinal_to_all_listings.ts @@ -0,0 +1,89 @@ +#!/usr/bin/env tsx +// Temporary updater for assigning elements `ordinal` to elements documents which do not have one +import { compact, property, sortBy } from 'lodash-es' +import { getElementsByListings } from '#controllers/listings/lib/elements' +import dbFactory from '#db/couchdb/base' +import { isNonEmptyArray } from '#lib/boolean_validations' +import { wait } from '#lib/promises' +import { updateElementDoc } from '#models/element' +import { shellExec } from '#scripts/scripts_utils' +import config from '#server/config' + +const dbBaseUrl = config.db.getOrigin() + +const assignElementsOrdinalToAllListings = async () => { + const getAllListingsIdsCurlCommand = `curl -H 'Content-Type:application/json' -H 'Accept: application/json' '${dbBaseUrl}/${config.db.name('lists')}/_all_docs?include_docs=true' | jq '.rows[] | .doc._id' | jq -s` + const { stdout } = await shellExec(getAllListingsIdsCurlCommand) + const allListingsIds = JSON.parse(stdout).slice(0, -2) // remove last array item `"_design/lists"` + await addListingsOrdinals(allListingsIds) + process.exit(0) +} + +function areSomeWithoutOrdinal (elements) { + return elements.find(el => (typeof (el.ordinal) === 'undefined')) +} + +async function updateOrdinals (elements) { + const orderedUris = elements.map(property('uri')) + return reorderElements(orderedUris, elements) +} + +const addListingOrdinals = async listingId => { + const elements = await getElementsByListings([ listingId ]) + if (isNonEmptyArray(elements)) { + if (areSomeWithoutOrdinal(elements)) { + console.log('##### 33 assign_elements_ordinal_to_all_listings.ts elements', elements) + // Just making sure + const sortedElements = sortBy(elements, 'created') + console.log('updating listing:', listingId) + return updateOrdinals(sortedElements) + } + } +} + +async function addListingsOrdinals (listingsIds) { + const remainingListingsIds = listingsIds.slice(0) // clone + const nextBatch = async () => { + // One by one since, updating all elements from a listing may be heavy + const batchListingsIds = remainingListingsIds.splice(0, 1) + if (batchListingsIds.length === 0) return + await Promise.all(compact(batchListingsIds.map(addListingOrdinals))) + // Give couchdb some rest + // await wait(1000) + return nextBatch() + } + await nextBatch() +} + +// Do not reuse reorderElements from controllers/listings/lib/elements.ts to be able to also update elements without ordinal. See reorderAndUpdateDocs below +const db = await dbFactory('elements') + +export async function reorderElements (uris, currentElements) { + const docsToUpdate = reorderAndUpdateDocs({ + updateDocFn: updateElementDoc, + newOrderedKeys: uris, + currentDocs: currentElements, + attributeToSortBy: 'uri', + indexKey: 'ordinal', + }) + if (docsToUpdate.length > 0) { + await db.bulk(docsToUpdate) + } +} + +function reorderAndUpdateDocs ({ updateDocFn, newOrderedKeys, currentDocs, attributeToSortBy, indexKey }) { + const docsToUpdate = [] + for (let i = 0; i < newOrderedKeys.length; i++) { + const currentDoc = currentDocs.find(el => el[attributeToSortBy] === newOrderedKeys[i]) + // Next line is different than in reorderAndUpdateDocs + // to be able to assign new `ordinal` key + if (currentDoc[indexKey] === undefined || currentDoc[indexKey] !== i) { + const newAttributes = {} + newAttributes[indexKey] = i + docsToUpdate.push(updateDocFn(newAttributes, currentDoc)) + } + } + return docsToUpdate +} + +assignElementsOrdinalToAllListings() diff --git a/server/controllers/listings/lib/elements.ts b/server/controllers/listings/lib/elements.ts index 0017b30b0..526b9f3e8 100644 --- a/server/controllers/listings/lib/elements.ts +++ b/server/controllers/listings/lib/elements.ts @@ -62,9 +62,35 @@ export async function bulkUpdateElements ({ oldElements, attribute, value }) { const elementsBulkUpdate = db.bulk +export async function reorderElements (uris, currentElements) { + const docsToUpdate = reorderAndUpdateDocs({ + updateDocFn: updateElementDoc, + newOrderedKeys: uris, + currentDocs: currentElements, + attributeToSortBy: 'uri', + indexKey: 'ordinal', + }) + if (docsToUpdate.length > 0) { + await elementsBulkUpdate(docsToUpdate) + } +} + function highestOrdinal (elements: ListingElement[]) { if (elements.length === 0) return -1 const highestOrdinalElement = maxBy(elements, 'ordinal') return highestOrdinalElement.ordinal } + +function reorderAndUpdateDocs ({ updateDocFn, newOrderedKeys, currentDocs, attributeToSortBy, indexKey }) { + const docsToUpdate = [] + for (let i = 0; i < newOrderedKeys.length; i++) { + const currentDoc = currentDocs.find(el => el[attributeToSortBy] === newOrderedKeys[i]) + if (currentDoc[indexKey] !== i) { + const newAttributes = {} + newAttributes[indexKey] = i + docsToUpdate.push(updateDocFn(newAttributes, currentDoc)) + } + } + return docsToUpdate +} diff --git a/server/controllers/listings/lib/listings.ts b/server/controllers/listings/lib/listings.ts index b70aa0fde..43ee051c5 100644 --- a/server/controllers/listings/lib/listings.ts +++ b/server/controllers/listings/lib/listings.ts @@ -1,4 +1,4 @@ -import { groupBy, map, pick } from 'lodash-es' +import { groupBy, map, pick, difference } from 'lodash-es' import { getEntitiesByUris } from '#controllers/entities/lib/get_entities_by_uris' import { getElementsByListings, createListingElements, deleteListingsElements } from '#controllers/listings/lib/elements' import { filterFoundElementsUris } from '#controllers/listings/lib/helpers' @@ -28,7 +28,6 @@ export async function getListingsByIdsWithElements (ids: ListingId[]) { if (!isNonEmptyArray(listings)) return [] const listingIds = map(listings, '_id') const elements = await getElementsByListings(listingIds) - if (!isNonEmptyArray(listings)) return [] const elementsByListing: ElementsByListing = groupBy(elements, 'list') listings.forEach(assignElementsToListing(elementsByListing)) return listings as ListingWithElements[] @@ -54,14 +53,15 @@ export async function updateListingAttributes (params) { export const bulkDeleteListings = db.bulkDelete export async function addListingElements ({ listing, uris, userId }: { listing: ListingWithElements, uris: EntityUri, userId: UserId }) { - const currentElements = listing.elements + const currentElements = listing.elements || [] const { foundElements, notFoundUris } = filterFoundElementsUris(currentElements, uris) await validateExistingEntities(notFoundUris) const { docs: createdElements } = await createListingElements({ uris: notFoundUris, listing, userId }) + const res = { ok: true, createdElements } if (isNonEmptyArray(foundElements)) { - return { ok: true, alreadyInList: foundElements, createdElements } + return Object.assign(res, { alreadyInList: foundElements }) } - return { ok: true, createdElements } + return res } export function validateListingsOwnership (userId: UserId, listings: Listing[]) { @@ -72,6 +72,17 @@ export function validateListingsOwnership (userId: UserId, listings: Listing[]) } } +export const validateElementsUrisInListing = (uris, listingElements) => { + if (listingElements.length === 0) return true + const listingElementsUris = map(listingElements, 'uri') + // truncate array to allow more performant validation on elements subset + listingElementsUris.length = uris.length + const urisNotInListing = difference(listingElementsUris, uris) + if (urisNotInListing.length > 0) { + throw newError('some elements are not in the list', 400, { uris: urisNotInListing }) + } +} + export async function getListingWithElements (listingId: ListingId) { const listings = await getListingsByIdsWithElements([ listingId ]) return listings[0] @@ -86,7 +97,8 @@ export async function deleteUserListingsAndElements (userId: UserId) { } const assignElementsToListing = (elementsByListing: ElementsByListing) => listing => { - listing.elements = elementsByListing[listing._id] || [] + const listingElements = elementsByListing[listing._id] || [] + listing.elements = listingElements.sort((a, b) => a.ordinal - b.ordinal) } async function validateExistingEntities (uris: EntityUri[]) { diff --git a/server/controllers/listings/listings.ts b/server/controllers/listings/listings.ts index c3dcf3d6b..20de266bd 100644 --- a/server/controllers/listings/listings.ts +++ b/server/controllers/listings/listings.ts @@ -7,6 +7,7 @@ import byIds from './by_ids.js' import create from './create.js' import deleteByIds from './delete_by_ids.js' import removeElements from './remove_elements.js' +import reorder from './reorder.js' import update from './update.js' export default { @@ -24,6 +25,7 @@ export default { 'add-elements': addElements, 'remove-elements': removeElements, delete: deleteByIds, + reorder, }, }), put: ActionsControllers({ diff --git a/server/controllers/listings/reorder.ts b/server/controllers/listings/reorder.ts new file mode 100644 index 000000000..15a0a3e8c --- /dev/null +++ b/server/controllers/listings/reorder.ts @@ -0,0 +1,30 @@ +import { reorderElements } from '#controllers/listings/lib/elements' +import { getListingWithElements, validateListingsOwnership, validateElementsUrisInListing } from '#controllers/listings/lib/listings' +import { newError } from '#lib/error/error' + +const sanitization = { + id: {}, + uris: {}, +} + +const controller = async ({ id, uris, reqUserId }) => { + const listing = await getListingWithElements(id) + validateListingsOwnership(reqUserId, [ listing ]) + const elements = listing.elements + if (!elements || elements.length === 0) { + throw newError('no elements to reorder', 400, { list: listing }) + } + validateElementsUrisInListing(uris, elements) + await reorderElements(uris, elements) + const resListing = await getListingWithElements(id) + return { + ok: true, + list: resListing, + } +} + +export default { + sanitization, + controller, + track: [ 'lists', 'reorder' ], +} diff --git a/server/models/element.ts b/server/models/element.ts index eea23db3d..9b6e9206d 100644 --- a/server/models/element.ts +++ b/server/models/element.ts @@ -17,8 +17,10 @@ const attributes = { validAtCreation: [ 'list', 'uri', + 'ordinal', ], updatable: [ + 'ordinal', 'uri', ], } diff --git a/tests/api/fixtures/listings.ts b/tests/api/fixtures/listings.ts index e16265ea5..8336850d6 100644 --- a/tests/api/fixtures/listings.ts +++ b/tests/api/fixtures/listings.ts @@ -50,3 +50,14 @@ export const createElement = async ({ visibility = [ 'public' ], uri, listing }, uri, } } + +export const removeElement = async ({ uri, listing }, userPromise) => { + userPromise = userPromise || getUser() + const user = await userPromise + const removeElements = '/api/lists?action=remove-elements' + + return customAuthReq(user, 'post', removeElements, { + id: listing._id, + uris: [ uri ], + }) +} diff --git a/tests/api/listings/ordinal.test.ts b/tests/api/listings/ordinal.test.ts index a66bcddba..aa925f824 100644 --- a/tests/api/listings/ordinal.test.ts +++ b/tests/api/listings/ordinal.test.ts @@ -1,4 +1,11 @@ -import { createListingWithElements } from '../fixtures/listings.js' +import { map } from 'lodash-es' +import { getByIdWithElements } from '#tests/api/utils/listings' +import { shouldNotBeCalled, rethrowShouldNotBeCalledErrors } from '#tests/unit/utils' +import { createListing, createListingWithElements, createElement, removeElement } from '../fixtures/listings.js' +import { authReq } from '../utils/utils.js' + +const endpoint = '/api/lists?action=' +const reorder = `${endpoint}reorder` describe('element:listing-ordinal', () => { it('should create elements with a listingId', async () => { @@ -6,4 +13,79 @@ describe('element:listing-ordinal', () => { listing.elements[0].ordinal.should.equal(0) listing.elements[1].ordinal.should.equal(1) }) + + it('should reject without elements to reorder', async () => { + const { listing } = await createListingWithElements() + try { + await authReq('post', reorder, { id: listing._id }) + .then(shouldNotBeCalled) + } catch (err) { + rethrowShouldNotBeCalledErrors(err) + err.body.status_verbose.should.equal('missing parameter in body: uris') + err.statusCode.should.equal(400) + } + }) + + it('should reject listing without elements', async () => { + const { listing } = await createListing() + const { element } = await createElement({}) + try { + await authReq('post', reorder, { + id: listing._id, + uris: [ element.uri ], + }) + .then(shouldNotBeCalled) + } catch (err) { + rethrowShouldNotBeCalledErrors(err) + err.body.status_verbose.should.equal('no elements to reorder') + err.statusCode.should.equal(400) + } + }) + + it('should reject with elements not belonging to a listing', async () => { + const { listing } = await createListingWithElements() + const { element } = await createElement({}) + try { + await authReq('post', reorder, { + id: listing._id, + uris: [ element.uri ], + }) + .then(shouldNotBeCalled) + } catch (err) { + rethrowShouldNotBeCalledErrors(err) + err.body.status_verbose.should.equal('some elements are not in the list') + err.statusCode.should.equal(400) + } + }) + + it('should reorder elements', async () => { + const { listing } = await createListingWithElements() + const [ uri1, uri2 ] = map(listing.elements, 'uri') + await authReq('post', reorder, { + id: listing._id, + uris: [ uri2, uri1 ], + }) + const resListing = await getByIdWithElements({ id: listing._id }) + resListing.elements[0].uri.should.equal(uri2) + resListing.elements[1].ordinal.should.equal(1) + }) + + it('should assign new ordinal if one element has been removed', async () => { + const { listing, uris } = await createListingWithElements() + const [ uri1, uri2 ] = uris + const { uri: uri3 } = await createElement({ listing }) + await removeElement({ uri: uri2, listing }) + + const resListing = await getByIdWithElements({ id: listing._id }) + resListing.elements[1].uri.should.equal(uri3) + resListing.elements[1].ordinal.should.equal(2) + + await authReq('post', reorder, { + id: listing._id, + uris: [ uri1, uri3 ], + }) + const resListing2 = await getByIdWithElements({ id: listing._id }) + resListing2.elements[1].uri.should.equal(uri3) + resListing2.elements[1].ordinal.should.equal(1) + }) })