Merge branch 'main' into pro-4733-npm-workspace-support

* main:
  hotfix for logging non-Error exceptions from api routes
  typo
  works
  update confirm message
  replace --force with --confirm + remove prompts for hosted customer users
  add --force to skip prompt
  remove stray log
  use prompts and update changelog
  figure should reset the wrapper
  handle paragraph without texts
  add lint-fix and remove-empty-paragraph task to rich-text

CHANGELOG.md

@@ -4,10 +4,26 @@
 
 ### Adds
 
+* Add `@apostrophecms/rich-text-widget:lint-fix-figure` task to wrap text nodes in paragraph tags when next to figure tag. Figure tags are not valid children of paragraph tags.
+* Add `@apostrophecms/rich-text-widget:remove-empty-paragraph` task to remove empty paragraphs from all existing rich-texts.
 * Add ability for custom tiptap extensions to access the options passed to rich text widgets at the area level
 * Add support for [npm workspaces](https://docs.npmjs.com/cli/v10/configuring-npm/package-json#workspaces) dependencies. A workspace dependency can now be used as an Apostrophe module even if it is not a direct dependency of the Apostrophe project. Only direct workspaces dependencies of the Apostrophe project are supported, meaning this will only work with workspaces set in the Apostrophe project. Workspaces set in npm modules are not supported, please use [`bundle`](https://v3.docs.apostrophecms.org/reference/module-api/module-overview.html#bundle) instead. For instance, I have an Apostrophe project called `website`. `website` is set with two [npm workspaces](https://docs.npmjs.com/cli/v10/using-npm/workspaces), `workspace-a` & `workspace-b`. `workspace-a` `package.json` contains a module named `blog` as a dependency. `website` can reference `blog` as enabled in the `modules` configuration.
+* The actual invocation of `renderPageForModule` by the `sendPage` method of all modules has been
+factored out to `renderPage`, which is no longer deprecated. This provides a convenient override point
+for those who wish to substitute something else for Nunjucks or just wrap the HTML in a larger data
+structure. For consistent results, one might also choose to override the `renderWidget` and `render`
+methods of the `@apostrophecms/area` module, which are used to render content while editing.
+Thanks to Michelin for their support of this work.
 
-## 3.55.0
+## 3.55.1 (2023-09-11)
+
+### Fixes
+
+* The structured logging for API routes now responds properly if an API route throws a `string` as an exception, rather than
+a politely `Error`-derived object with a `stack` property. Previously this resulted in an error message about the logging
+system itself, which was not useful for debugging the original exception.
+
+## 3.55.0 (2023-08-30)
 
 ### Adds
 

modules/@apostrophecms/module/index.js

@@ -258,7 +258,7 @@ module.exports = {
             self.logError(req, `api-error${typeTrail}`, msg, {
               name: response.name,
               status: response.code,
-              stack: error.stack.split('\n').slice(1).map(line => line.trim()),
+              stack: (error.stack || '').split('\n').slice(1).map(line => line.trim()),
               errorPath: response.path,
               data: response.data
             });
@@ -418,13 +418,11 @@ module.exports = {
       // `data.query` (req.query)
       //
       // This method is async in 3.x and must be awaited.
+      //
+      // No longer deprecated because it is a useful override point
+      // for this part of the behavior of sendPage.
 
       async renderPage(req, template, data) {
-        // TODO Remove in next major version.
-        self.apos.util.warnDevOnce(
-          'deprecate-renderPage',
-          'self.renderPage() is deprecated. Use self.sendPage() instead.'
-        );
         return self.apos.template.renderPageForModule(req, template, data, self);
       },
 
@@ -482,9 +480,8 @@ module.exports = {
           try {
             await self.apos.page.emit('beforeSend', req);
             await self.apos.area.loadDeferredWidgets(req);
-            req.res.send(
-              await self.apos.template.renderPageForModule(req, template, data, self)
-            );
+            const result = await self.renderPage(req, template, data);
+            req.res.send(result);
             span.setStatus({ code: telemetry.api.SpanStatusCode.OK });
           } catch (err) {
             telemetry.handleError(span, err);

modules/@apostrophecms/rich-text-widget/index.js

@@ -739,5 +739,164 @@ module.exports = {
         return finalData;
       }
     };
+  },
+  tasks(self) {
+    const confirm = async (isConfirmed) => {
+      if (isConfirmed) {
+        return true;
+      }
+
+      console.log('This task will perform an update on all existing rich-text widget. You should manually backup your database before running this command in case it becomes necessary to revert the changes. You can add --confirm to the command to skip this message and run the command');
+
+      return false;
+    };
+
+    return {
+      'remove-empty-paragraph': {
+        usage: 'Usage: node app @apostrophecms/rich-text-widget:remove-empty-paragraph --confirm\n\nRemove empty paragraph. If a paragraph contains no visible text or only blank characters, it will be removed.\n',
+        task: async (argv) => {
+          const iterator = async (doc, widget, dotPath) => {
+            if (widget.type !== self.name) {
+              return;
+            }
+
+            const updates = {};
+            if (widget.content.includes('<p>')) {
+              const dom = cheerio.load(widget.content);
+              const paragraph = dom('body').find('p');
+
+              paragraph.each((index, element) => {
+                const isEmpty = /^(\s|&nbsp;)*$/.test(dom(element).text());
+                isEmpty && dom(element).remove();
+
+                if (isEmpty) {
+                  updates[dotPath] = {
+                    ...widget,
+                    content: dom('body').html()
+                  };
+                }
+              });
+            }
+
+            if (Object.keys(updates).length) {
+              await self.apos.doc.db.updateOne(
+                { _id: doc._id },
+                { $set: updates }
+              );
+              self.apos.util.log(`Document ${doc._id} rich-texts have been updated`);
+            }
+          };
+
+          const isConfirmed = await confirm(argv.confirm);
+
+          return isConfirmed && self.apos.migration.eachWidget({}, iterator);
+        }
+      },
+      'lint-fix-figure': {
+        usage: 'Usage: node app @apostrophecms/rich-text-widget:lint-fix-figure --confirm\n\nFigure tags is allowed inside paragraph. This task will look for figure tag next to empty paragraph and wrap the text node around inside paragraph.\n',
+        task: async (argv) => {
+          const blockNodes = [
+            'address',
+            'article',
+            'aside',
+            'blockquote',
+            'canvas',
+            'dd',
+            'div',
+            'dl',
+            'dt',
+            'fieldset',
+            'figcaption',
+            'figure',
+            'footer',
+            'form',
+            'h1',
+            'h2',
+            'h3',
+            'h4',
+            'h5',
+            'h6',
+            'header',
+            'hr',
+            'li',
+            'main',
+            'nav',
+            'noscript',
+            'ol',
+            'p',
+            'pre',
+            'section',
+            'table',
+            'tfoot',
+            'ul',
+            'video'
+          ];
+          const append = ({
+            dom,
+            wrapper,
+            element
+          }) => {
+            return wrapper
+              ? wrapper.append(element) && wrapper
+              : dom(element).wrap('<p></p>').parent();
+          };
+
+          const iterator = async (doc, widget, dotPath) => {
+            if (widget.type !== self.name) {
+              return;
+            }
+
+            const updates = {};
+            if (widget.content.includes('<figure')) {
+              const dom = cheerio.load(widget.content);
+              // reference: https://stackoverflow.com/questions/28855070/css-select-element-without-text-inside
+              const figure = dom('body').find('p:not(:has(:not(:empty)))+figure,figure+p:not(:has(:not(:empty)))');
+              const parent = figure.parent().contents();
+
+              let wrapper = null;
+
+              parent.each((index, element) => {
+                const isFigure = element.type === 'tag' && element.name === 'figure';
+                isFigure && (wrapper = null);
+
+                const isNonWhitespaceTextNode = element.type === 'text' && /^\s*$/.test(element.data) === false;
+                isNonWhitespaceTextNode && (wrapper = append({
+                  dom,
+                  wrapper,
+                  element
+                }));
+
+                const isInlineNode = element.type === 'tag' && blockNodes.includes(element.name) === false;
+                isInlineNode && (wrapper = append({
+                  dom,
+                  wrapper,
+                  element
+                }));
+
+                const hasUpdate = isNonWhitespaceTextNode || isInlineNode;
+                if (hasUpdate) {
+                  updates[dotPath] = {
+                    ...widget,
+                    content: dom('body').html()
+                  };
+                }
+              });
+            }
+
+            if (Object.keys(updates).length) {
+              await self.apos.doc.db.updateOne(
+                { _id: doc._id },
+                { $set: updates }
+              );
+              self.apos.util.log(`Document ${doc._id} rich-texts have been updated`);
+            }
+          };
+
+          const isConfirmed = await confirm(argv.confirm);
+
+          return isConfirmed && self.apos.migration.eachWidget({}, iterator);
+        }
+      }
+    };
   }
 };

package.json

@@ -1,6 +1,6 @@
 {
   "name": "apostrophe",
-  "version": "3.55.0",
+  "version": "3.55.1",
   "description": "The Apostrophe Content Management System.",
   "main": "index.js",
   "scripts": {

test/weird-api-errors.js

@@ -0,0 +1,81 @@
+const t = require('../test-lib/test.js');
+const assert = require('assert');
+
+describe('Don\'t crash on weird API errors', function() {
+
+  after(async function() {
+    return t.destroy(apos);
+  });
+
+  this.timeout(t.timeout);
+
+  let apos;
+
+  it('should initialize apos', async function() {
+    apos = await t.create({
+      root: module,
+      modules: {
+        'api-test': {
+          apiRoutes(self) {
+            return {
+              get: {
+                fetchItFine(req) {
+                  return {
+                    nifty: true
+                  };
+                },
+                fetchItFailWeird(req) {
+                  throw 'not-an-error-object';
+                },
+                fetchItFailNormal(req) {
+                  throw new Error('normal error');
+                }
+              }
+            };
+          }
+        }
+      }
+    });
+  });
+  it('should fetch fine in the normal case', async function() {
+    const body = await apos.http.get('/api/v1/api-test/fetch-it-fine', {});
+    assert(typeof body === 'object');
+    assert.strictEqual(body.nifty, true);
+  });
+  it('should fail politely in the weird case of a non-Error exception', async function() {
+    let msgWas;
+    const consoleError = console.error;
+    console.error = msg => {
+      msgWas = msg;
+    };
+    try {
+      await apos.http.get('/api/v1/api-test/fetch-it-fail-weird', {});
+      // Should not get here
+      assert(false);
+    } catch (e) {
+      // Make sure the logging system itself is not at fault
+      assert(!msgWas.toString().includes('Structured logging error'));
+    } finally {
+      console.error = consoleError;
+      console.error(msgWas);
+    }
+  });
+  it('should fail politely in the normal case of an Error exception', async function() {
+    let msgWas;
+    const consoleError = console.error;
+    console.error = msg => {
+      msgWas = msg;
+    };
+    try {
+      await apos.http.get('/api/v1/api-test/fetch-it-fail-normal', {});
+      // Should not get here
+      assert(false);
+    } catch (e) {
+      // Make sure the logging system itself is not at fault
+      assert(!msgWas.toString().includes('Structured logging error'));
+    } finally {
+      console.error = consoleError;
+      console.error(msgWas);
+    }
+  });
+});