Add edit functionality to shares

integ_tests/test_sharing.py

@@ -170,3 +170,65 @@ def test_share_journal_no_overrides(page: Page, radicale_server: str) -> None:
     expect(
         page.locator("tr[data-name='sharetokenrowtemplate']:not(.hidden)")
     ).to_have_count(1)
+
+
+def test_edit_share_by_token(page: Page, radicale_server: str) -> None:
+    login(page, radicale_server)
+    create_collection(page, radicale_server)
+    page.hover("article:not(.hidden)")
+    page.click('article:not(.hidden) a[data-name="share"]', force=True, strict=True)
+
+    # Create RO share
+    page.click('button[data-name="sharebytoken"]')
+    page.click('#newshare button[data-name="submit"]')
+    expect(
+        page.locator("tr[data-name='sharetokenrowtemplate']:not(.hidden) img[alt='RO']")
+    ).to_be_visible()
+
+    # Edit to RW
+    page.click('tr:not(.hidden) button[data-name="edit"]')
+    expect(page.locator('#newshare h1')).to_have_text("Edit Share")
+    page.click('label[for="newshare_attr_permissions_rw"]')
+    page.click('#newshare button[data-name="submit"]')
+
+    # Verify RW
+    expect(
+        page.locator("tr[data-name='sharetokenrowtemplate']:not(.hidden) img[alt='RW']")
+    ).to_be_visible()
+
+
+def test_edit_share_by_map(page: Page, radicale_server: str) -> None:
+    login(page, radicale_server)
+    create_collection(page, radicale_server)
+    page.hover("article:not(.hidden)")
+    page.click('article:not(.hidden) a[data-name="share"]', force=True, strict=True)
+
+    # Create RO map share
+    page.click('button[data-name="sharebymap"]')
+    page.locator('input[data-name="shareuser"]').fill("max")
+    page.locator('input[data-name="sharehref"]').fill("mapped")
+    page.click('#newshare button[data-name="submit"]')
+    expect(
+        page.locator("tr[data-name='sharemaprowtemplate']:not(.hidden) img[alt='RO']")
+    ).to_be_visible()
+
+    # Edit map share
+    page.click('tr:not(.hidden) button[data-name="edit"]')
+    expect(page.locator('#newshare h1')).to_have_text("Edit Share")
+    expect(page.locator('input[data-name="shareuser"]')).to_be_disabled()
+    expect(page.locator('input[data-name="sharehref"]')).to_be_disabled()
+
+    # Change permissions and enabled status
+    page.click('label[for="newshare_attr_permissions_rw"]')
+    page.uncheck('input[data-name="enabled"]')
+    page.click('#newshare button[data-name="submit"]')
+
+    # Verify changes
+    expect(
+        page.locator("tr[data-name='sharemaprowtemplate']:not(.hidden) img[alt='RW']")
+    ).to_be_visible()
+    # If disabled, it might not show up or show differently, but our current UI doesn't visually distinguish enabled/disabled in the list yet
+    # Let's verify by re-opening edit scene
+    page.click('tr:not(.hidden) button[data-name="edit"]')
+    expect(page.locator('input[data-name="enabled"]')).not_to_be_checked()
+    page.click('#newshare button[data-name="cancel"]')

radicale/web/internal_data/index.html

@@ -153,17 +153,19 @@
           <tbody>
             <tr data-name="sharetokenrowtemplate" class="hidden">
               <td>
-                <button type="button" class="red inline" data-name="delete"><img src="css/icons/delete.svg"
+                <button type="button" class="blue inline" data-name="edit"><img src="css/icons/edit.svg"
-                    class="small_icon" alt="Delete"></button>
+                    class="small_icon" alt="Edit"></button>
               </td>
               <td><img src="css/icons/edit.svg" class="med_icon" alt="RW" data-name="rw" /><img src="css/icons/eye.svg"
                   class="med_icon" alt="RO" data-name="ro" /></td>
               <td><input type="text" data-name="pathortoken" value="" readonly=""
                   onfocus="this.setSelectionRange(0, 99999);" class="inline"></td>
+              <td>
+                <button type="button" class="red inline" data-name="delete"><img src="css/icons/delete.svg"
+                    class="small_icon" alt="Delete"></button>
+              </td>
             </tr>
             <tr>
-              <td></td>
-              <td></td>
               <td><button type="button" class="blue inline" data-name="sharebytoken"><img src="css/icons/new.svg"
                     class="small_icon" alt="New Share by Token"></button>
               </td>
@@ -177,17 +179,19 @@
           <tbody>
             <tr data-name="sharemaprowtemplate" class="hidden">
               <td>
-                <button type="button" class="red inline" data-name="delete"><img src="css/icons/delete.svg"
+                <button type="button" class="blue inline" data-name="edit"><img src="css/icons/edit.svg"
-                    class="small_icon" alt="Delete"></button>
+                    class="small_icon" alt="Edit"></button>
               </td>
               <td><img src="css/icons/edit.svg" class="med_icon" alt="RW" data-name="rw" /><img src="css/icons/eye.svg"
                   class="med_icon" alt="RO" data-name="ro" /></td>
               <td><input type="text" data-name="pathortoken" value="" readonly=""
                   onfocus="this.setSelectionRange(0, 99999);" class="inline"></td>
+              <td>
+                <button type="button" class="red inline" data-name="delete"><img src="css/icons/delete.svg"
+                    class="small_icon" alt="Delete"></button>
+              </td>
             </tr>
             <tr>
-              <td></td>
-              <td></td>
               <td>
                 <button type="button" class="blue inline" data-name="sharebymap">
                   <img src="css/icons/new.svg" class="small_icon" alt="New Share by Map">

radicale/web/internal_data/js/api/sharing.js

@@ -118,22 +118,56 @@ export function discover_server_features(user, password, callback) {
     );
 }
 
-/**
+export class Share {
- * @typedef {Object} Share
+    /**
- * @property {string} ShareType
+     * @param {Object} [data]
- * @property {string} PathOrToken
+     */
- * @property {string} PathMapped
+    constructor(data = {}) {
- * @property {string} Owner
+        /** @type {string} */ this.ShareType = data.ShareType || "";
- * @property {string} User
+        /** @type {string} */ this.PathOrToken = data.PathOrToken || "";
- * @property {string} Permissions
+        /** @type {string} */ this.PathMapped = data.PathMapped || "";
- * @property {boolean} EnabledByOwner
+        /** @type {string} */ this.Owner = data.Owner || "";
- * @property {boolean} EnabledByUser
+        /** @type {string} */ this.User = data.User || "";
- * @property {boolean} HiddenByOwner
+        /** @type {string} */ this.Permissions = data.Permissions || "r";
- * @property {boolean} HiddenByUser
+        /** @type {boolean} */ this.EnabledByOwner = data.EnabledByOwner ?? data.Enabled ?? false;
- * @property {number} TimestampCreated
+        /** @type {boolean} */ this.EnabledByUser = data.EnabledByUser ?? data.Enabled ?? false;
- * @property {number} TimestampUpdated
+        /** @type {boolean} */ this.HiddenByOwner = data.HiddenByOwner ?? data.Hidden ?? false;
- * @property {string} Properties
+        /** @type {boolean} */ this.HiddenByUser = data.HiddenByUser ?? data.Hidden ?? false;
- */
+        /** @type {number} */ this.TimestampCreated = data.TimestampCreated || 0;
+        /** @type {number} */ this.TimestampUpdated = data.TimestampUpdated || 0;
+        /** @type {Object} */ this.Properties = data.Properties || {};
+    }
+
+    /**
+     * @returns {boolean}
+     */
+    get Enabled() {
+        return this.EnabledByOwner || this.EnabledByUser;
+    }
+
+    /**
+     * @param {boolean} value
+     */
+    set Enabled(value) {
+        this.EnabledByOwner = value;
+        this.EnabledByUser = value;
+    }
+
+    /**
+     * @returns {boolean}
+     */
+    get Hidden() {
+        return this.HiddenByOwner || this.HiddenByUser;
+    }
+
+    /**
+     * @param {boolean} value
+     */
+    set Hidden(value) {
+        this.HiddenByOwner = value;
+        this.HiddenByUser = value;
+    }
+}
 
 /**
  * @param {string} user
@@ -149,7 +183,8 @@ export function reload_sharing_list(user, password, collection, callback) {
         { PathMapped: collection.href },
         function (response) {
             let parsed = JSON.parse(response);
-            callback(parsed["Content"] || [], null);
+            let shares = (parsed["Content"] || []).map(data => new Share(data));
+            callback(shares, null);
         },
         null, // on_not_found
         function (error) {
@@ -191,21 +226,13 @@ export function get_property_key(type, property) {
 /**
  * @param {string} user
  * @param {string} password
- * @param {string} pathMapped
+ * @param {Share} share
- * @param {string} permissions
- * @param {boolean} enabled
- * @param {boolean} hidden
- * @param {Object} properties
  * @param {function(?string):void} callback
  */
 export function add_share_by_token(
     user,
     password,
-    pathMapped,
+    share,
-    permissions,
-    enabled,
-    hidden,
-    properties,
     callback,
 ) {
     call_sharing_api(
@@ -213,11 +240,11 @@ export function add_share_by_token(
         password,
         "token/create",
         {
-            PathMapped: pathMapped,
+            PathMapped: share.PathMapped,
-            Permissions: permissions,
+            Permissions: share.Permissions,
-            Enabled: enabled,
+            Enabled: share.Enabled,
-            Hidden: hidden,
+            Hidden: share.Hidden,
-            Properties: properties,
+            Properties: share.Properties,
         },
         function (response) {
             let json_response = JSON.parse(response);
@@ -237,25 +264,13 @@ export function add_share_by_token(
 /**
  * @param {string} user
  * @param {string} password
- * @param {string} pathMapped
+ * @param {Share} share
- * @param {string} permissions
- * @param {boolean} enabled
- * @param {boolean} hidden
- * @param {Object} properties
- * @param {string} share_user
- * @param {string} href
  * @param {function(?string):void} callback
  */
 export function add_share_by_map(
     user,
     password,
-    pathMapped,
+    share,
-    permissions,
-    enabled,
-    hidden,
-    properties,
-    share_user,
-    href,
     callback,
 ) {
     call_sharing_api(
@@ -263,13 +278,13 @@ export function add_share_by_map(
         password,
         "map/create",
         {
-            PathMapped: pathMapped,
+            PathMapped: share.PathMapped,
-            Permissions: permissions,
+            Permissions: share.Permissions,
-            Enabled: enabled,
+            Enabled: share.Enabled,
-            Hidden: hidden,
+            Hidden: share.Hidden,
-            Properties: properties,
+            Properties: share.Properties,
-            User: share_user,
+            User: share.User,
-            PathOrToken: "/" + share_user + "/" + href,
+            PathOrToken: share.PathOrToken,
         },
         function (response) {
             let json_response = JSON.parse(response);
@@ -289,20 +304,20 @@ export function add_share_by_map(
 /**
  * @param {string} user
  * @param {string} password
- * @param {string} token
+ * @param {Share} share
  * @param {function(?string):void} callback
  */
 export function delete_share_by_token(
     user,
     password,
-    token,
+    share,
     callback,
 ) {
     call_sharing_api(
         user,
         password,
         "token/delete",
-        { PathOrToken: token },
+        { PathOrToken: share.PathOrToken },
         function (response) {
             let json_response = JSON.parse(response);
             if (json_response["Status"] !== "success") {
@@ -321,20 +336,97 @@ export function delete_share_by_token(
 /**
  * @param {string} user
  * @param {string} password
- * @param {string} pathortoken
+ * @param {Share} share
  * @param {function(?string):void} callback
  */
 export function delete_share_by_map(
     user,
     password,
-    pathortoken,
+    share,
     callback,
 ) {
     call_sharing_api(
         user,
         password,
         "map/delete",
-        { PathOrToken: pathortoken },
+        { PathOrToken: share.PathOrToken },
+        function (response) {
+            let json_response = JSON.parse(response);
+            if (json_response["Status"] !== "success") {
+                callback(json_response["Status"] || "Unknown error");
+            } else {
+                callback(null);
+            }
+        },
+        null,
+        function (error) {
+            callback(error);
+        }
+    );
+}
+/**
+ * @param {string} user
+ * @param {string} password
+ * @param {Share} share
+ * @param {function(?string):void} callback
+ */
+export function update_share_by_token(
+    user,
+    password,
+    share,
+    callback,
+) {
+    call_sharing_api(
+        user,
+        password,
+        "token/update",
+        {
+            PathOrToken: share.PathOrToken,
+            Permissions: share.Permissions,
+            Enabled: share.Enabled,
+            Hidden: share.Hidden,
+            Properties: share.Properties,
+        },
+        function (response) {
+            let json_response = JSON.parse(response);
+            if (json_response["Status"] !== "success") {
+                callback(json_response["Status"] || "Unknown error");
+            } else {
+                callback(null);
+            }
+        },
+        null,
+        function (error) {
+            callback(error);
+        }
+    );
+}
+
+/**
+ * @param {string} user
+ * @param {string} password
+ * @param {Share} share
+ * @param {function(?string):void} callback
+ */
+export function update_share_by_map(
+    user,
+    password,
+    share,
+    callback,
+) {
+    call_sharing_api(
+        user,
+        password,
+        "map/update",
+        {
+            PathOrToken: share.PathOrToken,
+            PathMapped: share.PathMapped,
+            User: share.User,
+            Permissions: share.Permissions,
+            Enabled: share.Enabled,
+            Hidden: share.Hidden,
+            Properties: share.Properties,
+        },
         function (response) {
             let json_response = JSON.parse(response);
             if (json_response["Status"] !== "success") {

radicale/web/internal_data/js/scenes/CreateEditShareScene.js

@@ -19,7 +19,7 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-import { add_share_by_map, add_share_by_token, get_property_key } from "../api/sharing.js";
+import { Share, add_share_by_map, add_share_by_token, get_property_key, update_share_by_map, update_share_by_token } from "../api/sharing.js";
 import { CollectionType } from "../models/collection.js";
 import { ErrorHandler } from "../utils/error.js";
 import { FormValidator, validate_href, validate_non_empty } from "../utils/form_validator.js";
@@ -29,15 +29,17 @@ import { Scene, pop_scene, scene_stack } from "./scene_manager.js";
 /**
  * @implements {Scene}
  */
-export class NewShareScene {
+export class CreateEditShareScene {
     /**
      * @param {string} user
      * @param {string} password
      * @param {import("../models/collection.js").Collection} collection
      * @param {string} shareType
      * @param {function():void} onclose
+     * @param {Share} [share] If provided, the scene will be in edit mode.
      */
-    constructor(user, password, collection, shareType, onclose) {
+    constructor(user, password, collection, shareType, onclose, share) {
+        let edit = !!share;
         let pathMapped = collection.href;
         /** @type {HTMLElement} */ let html_scene = document.getElementById("newshare");
         /** @type {HTMLFormElement} */ let form = html_scene.querySelector("form");
@@ -56,6 +58,7 @@ export class NewShareScene {
         /** @type {HTMLInputElement} */ let color_override_input = html_scene.querySelector("[data-name=color_override]");
 
         /** @type {HTMLElement} */ let error_form = html_scene.querySelector("[data-name=error]");
+        /** @type {HTMLElement} */ let submit_btn = html_scene.querySelector("[data-name=submit]");
         /** @type {HTMLElement} */ let cancel_btn = html_scene.querySelector("[data-name=cancel]");
 
         let errorHandler = new ErrorHandler(error_form);
@@ -120,12 +123,29 @@ export class NewShareScene {
                     }
                 };
 
-                if (shareType === "map") {
+                let new_share = new Share({
-                    let share_user = shareuser_input.value;
+                    ShareType: shareType,
-                    let href = sharehref_input.value;
+                    PathMapped: pathMapped,
-                    add_share_by_map(user, password, pathMapped, permissions, enabled, hidden, properties, share_user, href, callback);
+                    Permissions: permissions,
+                    Enabled: enabled,
+                    Hidden: hidden,
+                    Properties: properties,
+                    User: edit ? share.User : shareuser_input.value,
+                    PathOrToken: edit ? share.PathOrToken : (shareType === "map" ? "/" + shareuser_input.value + "/" + sharehref_input.value : ""),
+                });
+
+                if (edit) {
+                    if (shareType === "map") {
+                        update_share_by_map(user, password, new_share, callback);
+                    } else {
+                        update_share_by_token(user, password, new_share, callback);
+                    }
                 } else {
-                    add_share_by_token(user, password, pathMapped, permissions, enabled, hidden, properties, callback);
+                    if (shareType === "map") {
+                        add_share_by_map(user, password, new_share, callback);
+                    } else {
+                        add_share_by_token(user, password, new_share, callback);
+                    }
                 }
             } catch (err) {
                 console.error(err);
@@ -140,20 +160,44 @@ export class NewShareScene {
             cancel_btn.onclick = oncancel;
             form.onsubmit = onsubmit;
 
+            html_scene.querySelector("h1").textContent = edit ? "Edit Share" : "New Share";
+            submit_btn.textContent = edit ? "Save" : "Create";
 
-            shareuser_input.value = "";
+            shareuser_input.value = edit ? share.User : "";
-            enabled_checkbox.checked = true;
+            shareuser_input.disabled = edit;
-            hidden_checkbox.checked = false;
+            enabled_checkbox.checked = edit ? share.Enabled : true;
-            permissions_ro_radio.checked = true;
+            hidden_checkbox.checked = edit ? share.Hidden : false;
-            permissions_rw_radio.checked = false;
+            permissions_ro_radio.checked = edit ? share.Permissions.toLowerCase() === "r" : true;
+            permissions_rw_radio.checked = edit ? share.Permissions.toLowerCase() === "rw" : false;
 
-            description_override_enabled.checked = false;
+            let description = collection.description || "";
-            description_override_input.value = collection.description || "";
+            let color = collection.color || "#ffffff";
-            description_override_input.disabled = true;
+            let description_override_enabled_value = false;
+            let color_override_enabled_value = false;
 
-            color_override_enabled.checked = false;
+            if (edit && share.Properties) {
-            color_override_input.value = collection.color || "#ffffff";
+                let description_key = get_property_key(collection.type, "DESCRIPTION");
-            color_override_input.disabled = true;
+                if (description_key && share.Properties[description_key]) {
+                    description = share.Properties[description_key];
+                    description_override_enabled_value = true;
+                }
+                let color_key = get_property_key(collection.type, "COLOR");
+                if (color_key && share.Properties[color_key]) {
+                    color = share.Properties[color_key];
+                    if (color.length === 9 && color.endsWith("ff")) {
+                        color = color.substring(0, 7);
+                    }
+                    color_override_enabled_value = true;
+                }
+            }
+
+            description_override_enabled.checked = description_override_enabled_value;
+            description_override_input.value = description;
+            description_override_input.disabled = !description_override_enabled_value;
+
+            color_override_enabled.checked = color_override_enabled_value;
+            color_override_input.value = color;
+            color_override_input.disabled = !color_override_enabled_value;
 
             let is_calendar = CollectionType.is_subset(CollectionType.CALENDAR, collection.type);
             let is_addressbook = collection.type === CollectionType.ADDRESSBOOK;
@@ -164,7 +208,12 @@ export class NewShareScene {
             }
 
             if (shareType === "map") {
-                sharehref_input.value = random_uuid();
+                if (edit) {
+                    sharehref_input.value = share.PathOrToken.split("/").pop() || "";
+                } else {
+                    sharehref_input.value = random_uuid();
+                }
+                sharehref_input.disabled = edit;
                 sharemapfields.classList.remove("hidden");
                 map_validator.validate();
             } else {

radicale/web/internal_data/js/scenes/ShareCollectionScene.js

@@ -27,7 +27,7 @@ import {
 } from "../api/sharing.js";
 import { Collection } from "../models/collection.js";
 import { ErrorHandler } from "../utils/error.js";
-import { NewShareScene } from "./NewShareScene.js";
+import { CreateEditShareScene } from "./CreateEditShareScene.js";
 import { Scene, pop_scene, push_scene, scene_stack } from "./scene_manager.js";
 
 /**
@@ -73,17 +73,17 @@ export class ShareCollectionScene {
     }
 
     function onsharebytoken() {
-      let new_share_scene = new NewShareScene(user, password, collection, "token", function () {
+      let create_edit_share_scene = new CreateEditShareScene(user, password, collection, "token", function () {
         update_share_list(user, password, collection, errorHandler);
       });
-      push_scene(new_share_scene, false);
+      push_scene(create_edit_share_scene, false);
     }
 
     function onsharebymap() {
-      let new_share_scene = new NewShareScene(user, password, collection, "map", function () {
+      let create_edit_share_scene = new CreateEditShareScene(user, password, collection, "map", function () {
         update_share_list(user, password, collection, errorHandler);
       });
-      push_scene(new_share_scene, false);
+      push_scene(create_edit_share_scene, false);
     }
 
     this.show = function () {
@@ -167,7 +167,7 @@ function update_share_list(user, password, collection, errorHandler) {
  * @param {import('../api/sharing.js').Share} share 
  * @param {HTMLElement} template 
  * @param {string} delete_label 
- * @param {function(string, string, string, function(?string):void):void} delete_action 
+ * @param {function(string, string, import('../api/sharing.js').Share, function(?string):void):void} delete_action 
  * @param {ErrorHandler} errorHandler
  */
 function add_share_row_node(user, password, collection, share, template, delete_label, delete_action, errorHandler) {
@@ -193,6 +193,14 @@ function add_share_row_node(user, password, collection, share, template, delete_
     console.warn("Unknown permissions", permissions);
   }
 
+  /** @type {HTMLElement} */ let edit_btn = node.querySelector("[data-name=edit]");
+  edit_btn.onclick = function () {
+    let create_edit_share_scene = new CreateEditShareScene(user, password, collection, share.ShareType, function () {
+      update_share_list(user, password, collection, errorHandler);
+    }, share);
+    push_scene(create_edit_share_scene, false);
+  };
+
   /** @type {HTMLElement} */ let delete_btn = node.querySelector("[data-name=delete]");
   delete_btn.onclick = function () {
     if (!confirm("Are you sure you want to delete " + delete_label + " " + pathortoken + "?")) {
@@ -201,7 +209,7 @@ function add_share_row_node(user, password, collection, share, template, delete_
     delete_action(
       user,
       password,
-      pathortoken,
+      share,
       function (error) {
         if (error) {
           errorHandler.setError(error);