Fix for single string change-requests

README.md

@@ -104,7 +104,7 @@ mongoimport --db metadata --collection saml20_idp --type json --file identity-pr
 cd ./manage-server/src/test/resources/json
 curl -u sysadmin:secret -X POST -H 'Content-Type: application/json' -d '@change_request.json' 'https://manage.test2.surfconext.nl/manage/api/internal/change-requests'
 ```
-Or the other supported flavour: a incremental change
+Or the other supported flavour: an incremental change
 ```
 curl -u sysadmin:secret -X POST -H 'Content-Type: application/json' -d '@incremental_change_request.json' 'https://manage.test2.surfconext.nl/manage/api/internal/change-requests'
 curl -u sysadmin:secret -X POST -H 'Content-Type: application/json' -d '@incremental_change_request.json' 'http://localhost:8080/manage/api/internal/change-requests'

manage-gui/src/components/metadata/MetaDataChangeRequests.jsx

@@ -112,9 +112,12 @@ class MetaDataChangeRequests extends React.Component {
 
     applyPathUpdates = (pathUpdates, data, pathUpdateType) => {
         const newData = cloneDeep(data);
+
         Object.keys(pathUpdates).forEach(key => {
             const singleValue = pathUpdates[key];
-            if (typeof singleValue === "object" && (!newData[key] || !Array.isArray(newData[key])) &&
+            if (typeof singleValue === "string") {
+                newData[key] = singleValue;
+            } else if (typeof singleValue === "object" && (!newData[key] || !Array.isArray(newData[key])) &&
                 ["allowedEntities", "disableConsent", "stepupEntities", "mfaEntities", "allowedResourceServers",
                     "mfaEntities"].indexOf(key) === -1) {
                 if (pathUpdateType === "ADDITION") {
@@ -132,16 +135,26 @@ class MetaDataChangeRequests extends React.Component {
                         newData[key] = []
                     }
                     //first remove everything, could be an attribute value change like loa-level
-                    const newValue = newData[key].filter(entry => !value.some(ent => entry.name === ent.name));
-                    //this alters the order and confuses the differ
-                    newData[key] = newValue.concat(value);
-                    newData[key].sort((a, b) => a.name.localeCompare(b.name));
-                    if (!data[key]) {
+                    const newValue = newData[key];
+                    if (Array.isArray(newValue)) {
+                        const newValue = newValue.filter(entry => !value.some(ent => entry.name === ent.name));
+                        //this alters the order and confuses the differ
+                        newData[key] = newValue.concat(value);
+                        newData[key].sort((a, b) => a.name.localeCompare(b.name));
+                    }
+                    const currentValue = data[key];
+                    if (!currentValue) {
                         data[key] = []
                     }
-                    data[key].sort((a, b) => a.name.localeCompare(b.name));
+                    if (Array.isArray(currentValue)) {
+                        data[key].sort((a, b) => a.name.localeCompare(b.name));
+                    }
                 } else if (newData[key]) {
-                    newData[key] = newData[key].filter(entry => !value.some(ent => entry.name === ent.name));
+                    const newKey = newData[key];
+                    if (Array.isArray(newKey)) {
+                        newData[key] = newKey.filter(entry => !value.some(ent => entry.name === ent.name));
+                    }
+
                 }
             }
         });
@@ -156,7 +169,7 @@ class MetaDataChangeRequests extends React.Component {
         sortDict(newData);
         let diffs;
         if (request.incrementalChange) {
-            //we can not simple show the pathUpdate (e.g. newData) as this is not against the actual data
+            //we can not simply show the pathUpdate (e.g. newData) as this is not against the actual data
             const appliedData = this.applyPathUpdates(newData, data, request.pathUpdateType);
             diffs = this.differ.diff(data, appliedData);
         } else {
@@ -185,14 +198,17 @@ class MetaDataChangeRequests extends React.Component {
                 </thead>
                 <tbody>
                 <tr>
-                    <td className={"notes"} colSpan={5}>Summary: <span
+                    <td className={"notes"} colSpan={headers.length}>Summary: <span
                         className={"notes"}>{request.note ? request.note : "-"}</span></td>
                 </tr>
                 <tr>
                     <td>{new Date(request.created).toGMTString()}</td>
                     <td>{request.auditData.userName}</td>
-                    <td className={"incremental"}><CheckBox name={"incremental"} readOnly={true}
-                                                            value={request.incrementalChange || false}/></td>
+                    <td className={"incremental"}>
+                        <div className={"wrapper"}><CheckBox name={"incremental"} readOnly={true}
+                                                            value={request.incrementalChange || false}/>
+                        </div>
+                    </td>
                     <td><ReactJson src={this.requestToJson(request)} name="changeRequest" collapsed={true}/></td>
                     <td className="nope">
                         <div className="accept">
@@ -207,7 +223,7 @@ class MetaDataChangeRequests extends React.Component {
                     </td>
                 </tr>
                 <tr>
-                    <td><CheckBox name={`${i}`}
+                    <td colSpan={5}><CheckBox name={`${i}`}
                                   value={showDetail || false}
                                   info={I18n.t("changeRequests.toggleDetails")}
                                   onChange={() => this.toggleShowDetail(request)}/>

manage-gui/src/components/metadata/MetaDataChangeRequests.scss

@@ -88,7 +88,13 @@
                 padding: 10px;
 
                 &.incremental {
-                    text-align: center;
+                    .wrapper {
+                        display: flex;
+                    }
+
+                    .checkbox {
+                        margin: auto;
+                    }
                 }
 
                 &.nope {
@@ -98,6 +104,10 @@
                 &.notes span.notes {
                     font-style: italic;
                 }
+
+                .checkbox {
+                    display: flex;
+                }
             }
         }
 

manage-server/src/main/java/manage/hook/EmptyRevisionHook.java

@@ -26,6 +26,7 @@ public MetaData prePut(MetaData previous, MetaData newMetaData) {
         Map<String, Object> newData = newMetaData.getData();
         boolean eq = mapEquality(previousData, newData) && mapEquality(newData, previousData);
         if (eq) {
+            //we need a schema, does not matter for which entityType
             Schema schema = metaDataAutoConfiguration.schema(EntityType.RP.getType());
             throw new ValidationException(schema, "No data is changed. An update would result in an empty revision", "empty-revision");
         }

manage-server/src/main/java/manage/model/MetaData.java

@@ -113,7 +113,13 @@ public void merge(PathUpdates pathUpdates) {
             }
             if (pathUpdates.isIncrementalChange()) {
                 Object rawReferenceValue = ((Map) reference).get(property);
-                if (rawReferenceValue instanceof List || rawReferenceValue == null) {
+                if (rawReferenceValue instanceof String) {
+                    if (value == null) {
+                        ((Map) reference).remove(property);
+                    } else {
+                        ((Map) reference).put(property, value);
+                    }
+                } else if (rawReferenceValue instanceof List || rawReferenceValue == null) {
                     List<Map<String, Object>> referenceValue = (List<Map<String, Object>>) rawReferenceValue;
                     final List valueList = value instanceof Map ? List.of(value) : (List) value;
                     if (PathUpdateType.ADDITION.equals(pathUpdates.getPathUpdateType())) {

manage-server/src/test/java/manage/control/MetaDataControllerTest.java

@@ -1583,6 +1583,36 @@ public void changeRequestRemovalMultiple() {
         assertEquals(0, allowedEntities.size());
     }
 
+    @Test
+    public void changeRequestChangeEntityID() {
+        Map<String, Object> pathUpdates = Map.of("entityid","https://changed.org");
+        Map<String, Object> auditData = Map.of("user", "jdoe");
+
+        MetaDataChangeRequest changeRequest = new MetaDataChangeRequest(
+                "6", EntityType.IDP.getType(), "Because....", pathUpdates, auditData
+        );
+        changeRequest.setIncrementalChange(true);
+        changeRequest.setPathUpdateType(PathUpdateType.ADDITION);
+
+        Map results = given().auth().preemptive().basic("sp-portal", "secret")
+                .when()
+                .body(changeRequest)
+                .header("Content-type", "application/json")
+                .post("manage/api/internal/change-requests")
+                .as(Map.class);
+
+        given()
+                .when()
+                .contentType(ContentType.JSON)
+                .body(new ChangeRequest((String) results.get("id"), EntityType.IDP.getType(), "6", "Rev notes"))
+                .put("/manage/api/client/change-requests/accept")
+                .then()
+                .statusCode(200);
+
+        MetaData metaData = metaDataRepository.findById("6", EntityType.IDP.getType());
+        assertEquals("https://changed.org", metaData.getData().get("entityid"));
+    }
+
     @Test
     public void searchWithEntityCategoriesMultpleKeys() throws JsonProcessingException {
         Map<String, Object> searchOptions = readValueFromFile("/api/search_multiple_equal_keys.json");

manage-server/src/test/resources/json/entity_id_change_request.json

@@ -0,0 +1,13 @@
+{
+  "metaDataId" : "6",
+  "type" : "saml20_idp",
+  "incrementalChange": true,
+  "pathUpdateType": "REMOVAL",
+  "pathUpdates" : {
+    "entityid" : "https://changed.org"
+  },
+  "auditData" : {
+    "user" : "jdoe"
+  },
+  "note": "Optional note describing the reason for this change"
+}