EditDialog: osmApiAuth types + remove redundant fetchJson() (#759)

src/services/helpers.ts

@@ -6,14 +6,16 @@ import { join, roundedToDegUrl } from '../utils';
 import { PROJECT_URL } from './project';
 import { getIdFromShortener, getShortenerSlug } from './shortener';
 
-export const parseXmlString = (xmlString) => {
+export type Xml2JsDocument = Record<string, any>; // TODO very specific api :)
+
+export const parseStringToXml2Js = (xmlString: string) => {
   const parser = new xml2js.Parser({
     explicitArray: false,
     explicitCharkey: false,
     explicitRoot: false,
   });
 
-  return new Promise<any>((resolve, reject) => {
+  return new Promise<Xml2JsDocument>((resolve, reject) => {
     parser.parseString(xmlString, (err, result) => {
       if (err) {
         reject(err);
@@ -95,8 +97,12 @@ export const getImageSize = (url): Promise<ImageSize> =>
     imgElement.src = url;
   });
 
-export const stringifyDomXml = (itemXml) =>
-  isString(itemXml) ? itemXml : new XMLSerializer().serializeToString(itemXml);
+export const stringifyDomXml = (itemXml: Node) => {
+  if (isString(itemXml)) {
+    throw new Error('String given');
+  }
+  return new XMLSerializer().serializeToString(itemXml);
+};
 
 // TODO better mexico border + add Australia, New Zealand & South Africa
 const polygonUsCan = [[-143, 36], [-117, 32], [-96, 25], [-50, 19], [-56, 71], [-175, 70], [-143, 36]]; // prettier-ignore

src/services/osmApiAuth.ts

@@ -1,21 +1,21 @@
 import Cookies from 'js-cookie';
 import escape from 'lodash/escape';
-import { osmAuth } from 'osm-auth';
+import { osmAuth, OSMAuthXHROptions } from 'osm-auth';
 import { Feature, FeatureTags, OsmId, Position, SuccessInfo } from './types';
 import {
   buildXmlString,
   getFullOsmappLink,
   getOsmappLink,
   getUrlOsmId,
-  parseXmlString,
+  parseStringToXml2Js,
   prod,
   stringifyDomXml,
+  Xml2JsDocument,
 } from './helpers';
 import { join } from '../utils';
 import { clearFeatureCache } from './osmApi';
 import { isBrowser } from '../components/helpers';
 import { getLabel } from '../helpers/featureLabel';
-import { fetchJson } from './fetch';
 
 const PROD_CLIENT_ID = 'vWUdEL3QMBCB2O9q8Vsrl3i2--tcM34rKrxSHR9Vg68';
 
@@ -36,9 +36,9 @@ const auth = osmAuth({
 });
 const osmWebsite = prod ? 'https://www.openstreetmap.org' : TEST_SERVER;
 
-const authFetch = async (options) =>
-  new Promise<any>((resolve, reject) => {
-    auth.xhr(options, (err, details) => {
+const authFetch = async <T>(options: OSMAuthXHROptions): Promise<T> =>
+  new Promise<T>((resolve, reject) => {
+    auth.xhr(options, (err: any, details: T) => {
       if (err) {
         reject(err);
         return;
@@ -53,7 +53,7 @@ export type OsmUser = {
 };
 
 export const fetchOsmUser = async (): Promise<OsmUser> => {
-  const response = await authFetch({
+  const response = await authFetch<string>({
     method: 'GET',
     path: '/api/0.6/user/details.json',
   });
@@ -101,52 +101,52 @@ const getChangesetXml = ({ changesetComment, feature }) => {
 };
 
 const putChangeset = (content: string) =>
-  authFetch({
+  authFetch<string>({
     method: 'PUT',
     path: '/api/0.6/changeset/create',
-    options: { header: { 'Content-Type': 'text/xml; charset=utf-8' } },
+    headers: { 'Content-Type': 'text/xml; charset=utf-8' },
     content,
   });
 
 const putChangesetClose = (changesetId: string) =>
-  authFetch({
+  authFetch<void>({
     method: 'PUT',
     path: `/api/0.6/changeset/${changesetId}/close`,
   });
 
 const getItem = (apiId: OsmId) =>
-  authFetch({
+  authFetch<Node>({
     method: 'GET',
     path: `/api/0.6/${getUrlOsmId(apiId)}`,
   });
 
 const getItemHistory = (apiId: OsmId) =>
-  authFetch({
+  authFetch<Node>({
     method: 'GET',
     path: `/api/0.6/${getUrlOsmId(apiId)}/history`,
   });
 
 const putItem = (apiId: OsmId, content: string) =>
-  authFetch({
+  authFetch<void>({
     method: 'PUT',
     path: `/api/0.6/${getUrlOsmId(apiId)}`,
-    options: { header: { 'Content-Type': 'text/xml; charset=utf-8' } },
+    headers: { 'Content-Type': 'text/xml; charset=utf-8' },
     content,
   });
 
 const deleteItem = (apiId: OsmId, content: string) =>
-  authFetch({
+  authFetch<void>({
     method: 'DELETE',
     path: `/api/0.6/${getUrlOsmId(apiId)}`,
-    options: { header: { 'Content-Type': 'text/xml; charset=utf-8' } },
+    headers: { 'Content-Type': 'text/xml; charset=utf-8' },
     content,
   });
 
 const createItem = (content: string) =>
-  authFetch({
+  authFetch<string>({
     method: 'PUT',
     path: `/api/0.6/node/create`,
-    options: { header: { 'Content-Type': 'text/xml; charset=utf-8' } },
+    headers: { 'Content-Type': 'text/xml; charset=utf-8' },
     content,
   });
 
@@ -162,41 +162,42 @@ const putOrDeleteItem = async (
   }
 };
 
-const getItemOrLastHistoric = async (apiId: OsmId) => {
+const getItemOrLastHistoric = async (apiId: OsmId): Promise<Xml2JsDocument> => {
   try {
-    return await getItem(apiId);
+    const item = await getItem(apiId);
+    return await parseStringToXml2Js(stringifyDomXml(item));
   } catch (e) {
     // e is probably XMLHttpRequest
     if (e?.status !== 410) {
       throw e;
     }
 
-    // Mind that tags are fetched during feature fetch (osmApi#getOsmPromise()) and replaced after edit
+    // For undelete we return the latest "existing" version
     const itemHistory = await getItemHistory(apiId);
-    const xml = await parseXmlString(stringifyDomXml(itemHistory));
+    const xml = await parseStringToXml2Js(stringifyDomXml(itemHistory));
     const items = xml[apiId.type];
     const existingVersion = items[items.length - 2];
     const deletedVersion = items[items.length - 1];
     existingVersion.$.version = deletedVersion.$.version;
     xml[apiId.type] = existingVersion;
-    return buildXmlString(xml);
+    return xml;
   }
 };
 
-const getDescription = (isCancelled, feature) => {
+const getDescription = (toBeDeleted: boolean, feature: Feature) => {
   const undelete = feature.deleted;
-  const action = undelete ? 'Undeleted' : isCancelled ? 'Deleted' : 'Edited';
+  const action = undelete ? 'Undeleted' : toBeDeleted ? 'Deleted' : 'Edited';
   const { subclass } = feature.properties;
   const name = feature.tags.name || subclass || getUrlOsmId(feature.osmMeta);
   return `${action} ${name}`;
 };
 
 const getChangesetComment = (
   comment: string,
-  isCancelled: boolean,
+  toBeDeleted: boolean,
   feature: Feature,
 ) => {
-  const description = getDescription(isCancelled, feature);
+  const description = getDescription(toBeDeleted, feature);
   return join(comment, ' • ', `${description} #osmapp`);
 };
 
@@ -206,52 +207,58 @@ const getXmlTags = (newTags: FeatureTags) =>
     .map(([k, v]) => ({ $: { k, v } }));
 
 const updateItemXml = async (
-  item,
+  item: Xml2JsDocument,
   apiId: OsmId,
   changesetId: string,
   tags: FeatureTags,
   toBeDeleted: boolean,
 ) => {
-  const xml = await parseXmlString(stringifyDomXml(item));
-  xml[apiId.type].$.changeset = changesetId;
+  item[apiId.type].$.changeset = changesetId;
   if (!toBeDeleted) {
-    xml[apiId.type].tag = getXmlTags(tags);
+    item[apiId.type].tag = getXmlTags(tags);
   }
-  return buildXmlString(xml);
+  return buildXmlString(item);
 };
 
-export const editOsmFeature = async (
+const checkVersionUnchanged = (
+  freshItem: Xml2JsDocument,
+  apiId: OsmId,
   feature: Feature,
-  comment: string,
-  newTags: FeatureTags,
-  isCancelled: boolean,
-): Promise<SuccessInfo> => {
-  const newestVersion = await fetchJson(
-    `https://api.openstreetmap.org/api/0.6/${feature.osmMeta.type}/${feature.osmMeta.id}.json`,
-  ).then(({ elements }) => elements[0].version as number);
-  const loadedVersion = feature.osmMeta.version;
+) => {
+  if (apiId === TEST_OSM_ID) {
+    return;
+  }
 
-  if (loadedVersion !== newestVersion) {
+  const freshVersion = freshItem[apiId.type].$.version;
+  if (feature.osmMeta.version !== freshVersion) {
     throw new Error('The object has been updated, reload and try again');
   }
+};
 
+// TODO split to editOsmFeature and undeleteOsmFeature
+export const editOsmFeature = async (
+  feature: Feature,
+  comment: string,
+  newTags: FeatureTags,
+  toBeDeleted: boolean,
+): Promise<SuccessInfo> => {
   const apiId = prod ? feature.osmMeta : TEST_OSM_ID;
-  const changesetComment = getChangesetComment(comment, isCancelled, feature);
+  const freshItem = await getItemOrLastHistoric(apiId);
+  checkVersionUnchanged(freshItem, apiId, feature);
+
+  const changesetComment = getChangesetComment(comment, toBeDeleted, feature);
   const changesetXml = getChangesetXml({ changesetComment, feature });
   const changesetId = await putChangeset(changesetXml);
-  const item = await getItemOrLastHistoric(apiId);
 
-  // TODO use version from `feature` (we dont want to overwrite someones changes)
-  // TODO or at least just apply tags diff (see createNoteText)
   const newItem = await updateItemXml(
-    item,
+    freshItem,
     apiId,
     changesetId,
     newTags,
-    isCancelled,
+    toBeDeleted,
   );
 
-  await putOrDeleteItem(isCancelled, apiId, newItem);
+  await putOrDeleteItem(toBeDeleted, apiId, newItem);
   await putChangesetClose(changesetId);
 
   clearFeatureCache(feature.osmMeta);
@@ -269,7 +276,7 @@ const getNewItemXml = async (
   [lon, lat]: Position,
   newTags: FeatureTags,
 ) => {
-  const xml = await parseXmlString('<osm><node lat="x"/></osm>'); // TODO this is hackish
+  const xml = await parseStringToXml2Js('<osm><node lat="x"/></osm>'); // TODO this is hackish
   xml.node.$.changeset = changesetId;
   xml.node.$.lon = lon;
   xml.node.$.lat = lat;