fix oversights in add missing schema fields (#4736)

Author: boutell

modules/@apostrophecms/migration/lib/addMissingSchemaFields.js

@@ -35,12 +35,48 @@ module.exports = (self) => {
       }, async doc => {
         const changes = {};
         await self.addMissingSchemaFieldsFor(doc, schema, '', changes);
+        if (doc.type === '@apostrophecms/user') {
+          // In an abundance of caution we should leave permissions-related
+          // properties alone. The role will only be missing if transitioning
+          // from advanced permissions back to regular and there is a right
+          // way to do that (via a task) so we should not interfere. There
+          // should be no default for the password, but don't chance it;
+          // also leave advanced permission group data alone just for good measure
+          delete changes.password;
+          delete changes.role;
+          delete changes.groupsIds;
+          delete changes.groupsFields;
+        }
         if (Object.keys(changes).length > 0) {
           updates++;
+          const seen = new Set();
+          const keep = {};
+          for (const key of Object.keys(changes)) {
+            // MongoDB won't tolerate $set for both
+            // a parent key and a subkey, which makes sense
+            // because we set the entire value for the
+            // parent key anyway. Filter out subkeys
+            // when the parent key is also being set
+            const components = key.split('.');
+            let path = '';
+            let preempted = false;
+            for (const component of components) {
+              path = path ? `${path}.${component}` : component;
+              if (seen.has(path)) {
+                preempted = true;
+                break;
+              }
+            }
+            if (preempted) {
+              continue;
+            }
+            keep[key] = changes[key];
+            seen.add(key);
+          }
           return self.apos.doc.db.updateOne({
             _id: doc._id
           }, {
-            $set: changes
+            $set: keep
           });
         }
       });
@@ -54,8 +90,10 @@ module.exports = (self) => {
     addMissingSchemaFieldsFor(doc, schema, dotPath, changes) {
       for (const field of schema) {
         const newDotPath = dotPath ? `${dotPath}.${field.name}` : field.name;
-        // Supply the default
-        if (doc[field.name] === undefined) {
+        // Supply the default if a field is undefined, and also in the
+        // edge case where due to a past bug an object field was falsy
+        if ((doc[field.name] === undefined) ||
+          ((field.type === 'object') && !doc[field.name])) {
           // Only undefined should fall back here
           const def = klona((field.def === undefined) ? self.apos.schema.fieldTypes[field.type]?.def : field.def);
           if (def !== undefined) {
@@ -91,7 +129,10 @@ module.exports = (self) => {
           }
         } else if (field.type === 'relationship') {
           for (const [ key, item ] of Object.entries(doc[field.fieldsStorage] || {})) {
-            const itemPath = `${newDotPath}.${field.fieldsStorage}.${key}`;
+            // Careful, newDotPath contains the relationship name, we are storing
+            // in the fieldsStorage property for this relationship
+            const storageDotPath = dotPath ? `${dotPath}.${field.fieldsStorage}` : field.fieldsStorage;
+            const itemPath = `${storageDotPath}.${key}`;
             self.addMissingSchemaFieldsFor(item, field.schema, itemPath, changes);
           }
         }
@@ -101,7 +142,7 @@ module.exports = (self) => {
     getPropLists() {
       const schema = {};
       for (const [ name, module ] of Object.entries(self.apos.doc.managers)) {
-        if (!module.__meta.name) {
+        if (!module.__meta?.name) {
           // Just a placeholder for a type present in the
           // database but not found in the project code
           continue;