diff --git a/web/src/composebox_typeahead.js b/web/src/composebox_typeahead.js index 5a37d325ed..bfb0c3aad4 100644 --- a/web/src/composebox_typeahead.js +++ b/web/src/composebox_typeahead.js @@ -489,6 +489,7 @@ export const slash_commands = [ text: $t({defaultMessage: "/todo (Create a collaborative to-do list)"}), name: "todo", aliases: "", + placeholder: $t({defaultMessage: "Task list"}), }, ]; diff --git a/web/src/submessage.ts b/web/src/submessage.ts index f120cca76f..40c1afa50b 100644 --- a/web/src/submessage.ts +++ b/web/src/submessage.ts @@ -29,12 +29,21 @@ const poll_widget_extra_data_schema = z }) .nullable(); +export const todo_widget_extra_data_schema = z + .object({ + task_list_title: z.string().optional(), + }) + .nullable(); + const widget_data_event_schema = z.object({ sender_id: z.number(), data: z.discriminatedUnion("widget_type", [ z.object({widget_type: z.literal("poll"), extra_data: poll_widget_extra_data_schema}), z.object({widget_type: z.literal("zform"), extra_data: zform_widget_extra_data_schema}), - z.object({widget_type: z.literal("todo"), extra_data: z.null()}), + z.object({ + widget_type: z.literal("todo"), + extra_data: todo_widget_extra_data_schema, + }), ]), }); diff --git a/web/src/todo_widget.js b/web/src/todo_widget.js index d45e911130..757358811f 100644 --- a/web/src/todo_widget.js +++ b/web/src/todo_widget.js @@ -7,6 +7,7 @@ import * as blueslip from "./blueslip"; import {$t} from "./i18n"; import {page_params} from "./page_params"; import * as people from "./people"; +import {todo_widget_extra_data_schema} from "./submessage"; // Any single user should send add a finite number of tasks // to a todo list. We arbitrarily pick this value. @@ -16,8 +17,46 @@ export class TaskData { task_map = new Map(); my_idx = 1; - constructor({current_user_id}) { + constructor({ + message_sender_id, + current_user_id, + is_my_task_list, + task_list_title, + report_error_function, + }) { + this.message_sender_id = message_sender_id; this.me = current_user_id; + this.is_my_task_list = is_my_task_list; + // input_mode indicates if the task list title is being input currently + this.input_mode = is_my_task_list; // for now + this.report_error_function = report_error_function; + + if (task_list_title) { + this.set_task_list_title(task_list_title); + } else { + this.set_task_list_title($t({defaultMessage: "Task list"})); + } + } + + set_task_list_title(new_title) { + this.input_mode = false; + this.task_list_title = new_title; + } + + get_task_list_title() { + return this.task_list_title; + } + + set_input_mode() { + this.input_mode = true; + } + + clear_input_mode() { + this.input_mode = false; + } + + get_input_mode() { + return this.input_mode; } get_widget_data() { @@ -41,6 +80,36 @@ export class TaskData { } handle = { + new_task_list_title: { + outbound: (title) => { + const event = { + type: "new_task_list_title", + title, + }; + if (this.is_my_task_list) { + return event; + } + return undefined; + }, + + inbound: (sender_id, data) => { + // Only the message author can edit questions. + if (sender_id !== this.message_sender_id) { + this.report_error_function( + `user ${sender_id} is not allowed to edit the task list title`, + ); + return; + } + + if (typeof data.title !== "string") { + this.report_error_function("todo widget: bad type for inbound task list title"); + return; + } + + this.set_task_list_title(data.title); + }, + }, + new_task: { outbound: (task, desc) => { this.my_idx += 1; @@ -143,18 +212,118 @@ export class TaskData { } } -export function activate(opts) { - const $elem = opts.$elem; - const callback = opts.callback; - +export function activate({$elem, callback, extra_data, message}) { + const parse_result = todo_widget_extra_data_schema.safeParse(extra_data); + if (!parse_result.success) { + blueslip.warn("invalid todo extra data", parse_result.error.issues); + return; + } + const {data} = parse_result; + const {task_list_title = ""} = data || {}; + const is_my_task_list = people.is_my_user_id(message.sender_id); const task_data = new TaskData({ + message_sender_id: message.sender_id, current_user_id: people.my_current_user_id(), + is_my_task_list, + task_list_title, + report_error_function: blueslip.warn, }); - function render() { + function update_edit_controls() { + const has_title = $elem.find("input.todo-task-list-title").val().trim() !== ""; + $elem.find("button.todo-task-list-title-check").toggle(has_title); + } + + function render_task_list_title() { + const task_list_title = task_data.get_task_list_title(); + const input_mode = task_data.get_input_mode(); + const can_edit = is_my_task_list && !input_mode; + + $elem.find(".todo-task-list-title-header").toggle(!input_mode); + $elem.find(".todo-task-list-title-header").text(task_list_title); + $elem.find(".todo-edit-task-list-title").toggle(can_edit); + update_edit_controls(); + + $elem.find(".todo-task-list-title-bar").toggle(input_mode); + } + + function start_editing() { + task_data.set_input_mode(); + + const task_list_title = task_data.get_task_list_title(); + $elem.find("input.todo-task-list-title").val(task_list_title); + render_task_list_title(); + $elem.find("input.todo-task-list-title").trigger("focus"); + } + + function abort_edit() { + task_data.clear_input_mode(); + render_task_list_title(); + } + + function submit_task_list_title() { + const $task_list_title_input = $elem.find("input.todo-task-list-title"); + let new_task_list_title = $task_list_title_input.val().trim(); + const old_task_list_title = task_data.get_task_list_title(); + + // We should disable the button for blank task list title, + // so this is just defensive code. + if (new_task_list_title.trim() === "") { + new_task_list_title = old_task_list_title; + } + + // Optimistically set the task list title locally. + task_data.set_task_list_title(new_task_list_title); + render_task_list_title(); + + // If there were no actual edits, we can exit now. + if (new_task_list_title === old_task_list_title) { + return; + } + + // Broadcast the new task list title to our peers. + const data = task_data.handle.new_task_list_title.outbound(new_task_list_title); + callback(data); + } + + function build_widget() { const html = render_widgets_todo_widget(); $elem.html(html); + $elem.find("input.todo-task-list-title").on("keyup", (e) => { + e.stopPropagation(); + update_edit_controls(); + }); + + $elem.find("input.todo-task-list-title").on("keydown", (e) => { + e.stopPropagation(); + + if (e.key === "Enter") { + submit_task_list_title(); + return; + } + + if (e.key === "Escape") { + abort_edit(); + return; + } + }); + + $elem.find(".todo-edit-task-list-title").on("click", (e) => { + e.stopPropagation(); + start_editing(); + }); + + $elem.find("button.todo-task-list-title-check").on("click", (e) => { + e.stopPropagation(); + submit_task_list_title(); + }); + + $elem.find("button.todo-task-list-title-remove").on("click", (e) => { + e.stopPropagation(); + abort_edit(); + }); + $elem.find("button.add-task").on("click", (e) => { e.stopPropagation(); $elem.find(".widget-error").text(""); @@ -210,9 +379,11 @@ export function activate(opts) { task_data.handle_event(event.sender_id, event.data); } + render_task_list_title(); render_results(); }; - render(); + build_widget(); + render_task_list_title(); render_results(); } diff --git a/web/src/widgetize.ts b/web/src/widgetize.ts index 3c8f43c046..65ef7c8501 100644 --- a/web/src/widgetize.ts +++ b/web/src/widgetize.ts @@ -12,7 +12,12 @@ type ZFormExtraData = { choices: {type: string; reply: string; long_name: string; short_name: string}[]; }; -type WidgetExtraData = PollWidgetExtraData | ZFormExtraData | null; +// TODO: This TodoWidgetExtraData type should be moved to web/src/todo_widget.js when it will be migrated +type TodoWidgetExtraData = { + task_list_title?: string; +}; + +type WidgetExtraData = PollWidgetExtraData | TodoWidgetExtraData | ZFormExtraData | null; type WidgetOptions = { widget_type: string; diff --git a/web/styles/widgets.css b/web/styles/widgets.css index 95c21dd57e..3d2ef6b8ac 100644 --- a/web/styles/widgets.css +++ b/web/styles/widgets.css @@ -216,7 +216,8 @@ button { &.add-task, &.add-desc, &.poll-option, - &.poll-question { + &.poll-question, + &.todo-task-list-title { padding: 4px 6px; margin: 2px 0; } @@ -228,7 +229,9 @@ button { } .poll-question-check, -.poll-question-remove { +.poll-question-remove, +.todo-task-list-title-check, +.todo-task-list-title-remove { width: 28px !important; height: 28px; border-radius: 3px; @@ -240,7 +243,8 @@ button { } } -.poll-edit-question { +.poll-edit-question, +.todo-edit-task-list-title { opacity: 0.4; display: inline-block; margin-left: 5px; @@ -250,7 +254,8 @@ button { } } -.poll-question-header { +.poll-question-header, +.todo-task-list-title-header { display: inline; } diff --git a/web/templates/widgets/todo_widget.hbs b/web/templates/widgets/todo_widget.hbs index 4311c4c7f7..114b1826cc 100644 --- a/web/templates/widgets/todo_widget.hbs +++ b/web/templates/widgets/todo_widget.hbs @@ -1,5 +1,11 @@
-

{{t "Task list" }}

+

{{t "Task list" }}

+ +
+ + + +
diff --git a/zerver/lib/validator.py b/zerver/lib/validator.py index f2015d58b1..37c1c56864 100644 --- a/zerver/lib/validator.py +++ b/zerver/lib/validator.py @@ -547,7 +547,7 @@ def validate_poll_data(poll_data: object, is_widget_author: bool) -> None: raise ValidationError(f"Unknown type for poll data: {poll_data['type']}") -def validate_todo_data(todo_data: object) -> None: +def validate_todo_data(todo_data: object, is_widget_author: bool) -> None: check_dict([("type", check_string)])("todo data", todo_data) assert isinstance(todo_data, dict) @@ -575,6 +575,19 @@ def validate_todo_data(todo_data: object) -> None: checker("todo data", todo_data) return + if todo_data["type"] == "new_task_list_title": + if not is_widget_author: + raise ValidationError("You can't edit the task list title unless you are the author.") + + checker = check_dict_only( + [ + ("type", check_string), + ("title", check_string), + ] + ) + checker("todo data", todo_data) + return + raise ValidationError(f"Unknown type for todo data: {todo_data['type']}") diff --git a/zerver/lib/widget.py b/zerver/lib/widget.py index d3ae3ecabd..a0e3ff20a4 100644 --- a/zerver/lib/widget.py +++ b/zerver/lib/widget.py @@ -21,6 +21,56 @@ def get_widget_data(content: str) -> Tuple[Optional[str], Optional[str]]: return None, None +def parse_poll_extra_data(content: str) -> Any: + # This is used to extract the question from the poll command. + # The command '/poll question' will pre-set the question in the poll + lines = content.splitlines() + question = "" + options = [] + if lines and lines[0]: + question = lines.pop(0).strip() + for line in lines: + # If someone is using the list syntax, we remove it + # before adding an option. + option = re.sub(r"(\s*[-*]?\s*)", "", line.strip(), count=1) + if len(option) > 0: + options.append(option) + extra_data = { + "question": question, + "options": options, + } + return extra_data + + +def parse_todo_extra_data(content: str) -> Any: + # This is used to extract the task list title from the todo command. + # The command '/todo Title' will pre-set the task list title + lines = content.splitlines() + task_list_title = "" + if lines and lines[0]: + task_list_title = lines.pop(0).strip() + tasks = [] + for line in lines: + # If someone is using the list syntax, we remove it + # before adding a task. + task_data = re.sub(r"(\s*[-*]?\s*)", "", line.strip(), count=1) + if len(task_data) > 0: + # a task and its description (optional) are separated + # by the (first) `:` character + task_data_array = task_data.split(":", 1) + tasks.append( + { + "task": task_data_array[0].strip(), + "desc": task_data_array[1].strip() if len(task_data_array) > 1 else "", + } + ) + extra_data = { + "task_list_title": task_list_title, + "tasks": tasks, + } + return extra_data + + def get_extra_data_from_widget_type(content: str, widget_type: Optional[str]) -> Any: if widget_type == "poll": # This is used to extract the question from the poll command. @@ -41,7 +91,15 @@ def get_extra_data_from_widget_type(content: str, widget_type: Optional[str]) -> "options": options, } return extra_data - return None + else: + # This is used to extract the task list title from the todo command. + # The command '/todo Title' will pre-set the task list title + lines = content.splitlines() + task_list_title = "" + if lines and lines[0]: + task_list_title = lines.pop(0).strip() + extra_data = {"task_list_title": task_list_title} + return extra_data def do_widget_post_save_actions(send_request: SendMessageRequest) -> None: diff --git a/zerver/tests/test_widgets.py b/zerver/tests/test_widgets.py index ef43cabfc5..9a02ae6404 100644 --- a/zerver/tests/test_widgets.py +++ b/zerver/tests/test_widgets.py @@ -98,10 +98,14 @@ class WidgetContentTestCase(ZulipTestCase): self.assertEqual(get_widget_data(content=message), (None, None)) # Add positive checks for context - self.assertEqual(get_widget_data(content="/todo"), ("todo", None)) - self.assertEqual(get_widget_data(content="/todo ignore"), ("todo", None)) + self.assertEqual(get_widget_data(content="/todo"), ("todo", {"task_list_title": ""})) + self.assertEqual( + get_widget_data(content="/todo Title"), ("todo", {"task_list_title": "Title"}) + ) # Test tokenization on newline character - self.assertEqual(get_widget_data(content="/todo\nignore"), ("todo", None)) + self.assertEqual( + get_widget_data(content="/todo\nignore"), ("todo", {"task_list_title": ""}) + ) def test_explicit_widget_content(self) -> None: # Users can send widget_content directly on messages @@ -167,7 +171,24 @@ class WidgetContentTestCase(ZulipTestCase): expected_submessage_content = dict( widget_type="todo", - extra_data=None, + extra_data={"task_list_title": ""}, + ) + + submessage = SubMessage.objects.get(message_id=message.id) + self.assertEqual(submessage.msg_type, "widget") + self.assertEqual(orjson.loads(submessage.content), expected_submessage_content) + + content = "/todo Example Task List Title" + payload["content"] = content + result = self.api_post(sender, "/api/v1/messages", payload) + self.assert_json_success(result) + + message = self.get_last_message() + self.assertEqual(message.content, content) + + expected_submessage_content = dict( + widget_type="todo", + extra_data={"task_list_title": "Example Task List Title"}, ) submessage = SubMessage.objects.get(message_id=message.id) @@ -226,6 +247,55 @@ class WidgetContentTestCase(ZulipTestCase): self.assertEqual(submessage.msg_type, "widget") self.assertEqual(orjson.loads(submessage.content), expected_submessage_content) + def test_todo_command_extra_data(self) -> None: + sender = self.example_user("cordelia") + stream_name = "Verona" + # We test for leading spaces. + content = "/todo School Work" + + payload = dict( + type="stream", + to=orjson.dumps(stream_name).decode(), + topic="whatever", + content=content, + ) + result = self.api_post(sender, "/api/v1/messages", payload) + self.assert_json_success(result) + + message = self.get_last_message() + self.assertEqual(message.content, content) + + expected_submessage_content = dict( + widget_type="todo", + extra_data=dict( + task_list_title="School Work", + ), + ) + + submessage = SubMessage.objects.get(message_id=message.id) + self.assertEqual(submessage.msg_type, "widget") + self.assertEqual(orjson.loads(submessage.content), expected_submessage_content) + + # Now don't supply a task list title. + + content = "/todo" + payload["content"] = content + result = self.api_post(sender, "/api/v1/messages", payload) + self.assert_json_success(result) + + expected_submessage_content = dict( + widget_type="todo", + extra_data=dict( + task_list_title="", + ), + ) + + message = self.get_last_message() + self.assertEqual(message.content, content) + submessage = SubMessage.objects.get(message_id=message.id) + self.assertEqual(submessage.msg_type, "widget") + self.assertEqual(orjson.loads(submessage.content), expected_submessage_content) + def test_poll_permissions(self) -> None: cordelia = self.example_user("cordelia") hamlet = self.example_user("hamlet") @@ -255,6 +325,37 @@ class WidgetContentTestCase(ZulipTestCase): result = post(hamlet, dict(type="question", question="Tabs or spaces?")) self.assert_json_error(result, "You can't edit a question unless you are the author.") + def test_todo_permissions(self) -> None: + cordelia = self.example_user("cordelia") + hamlet = self.example_user("hamlet") + stream_name = "Verona" + content = "/todo School Work" + + payload = dict( + type="stream", + to=orjson.dumps(stream_name).decode(), + topic="whatever", + content=content, + ) + result = self.api_post(cordelia, "/api/v1/messages", payload) + self.assert_json_success(result) + + message = self.get_last_message() + + def post(sender: UserProfile, data: Dict[str, object]) -> "TestHttpResponse": + payload = dict( + message_id=message.id, msg_type="widget", content=orjson.dumps(data).decode() + ) + return self.api_post(sender, "/api/v1/submessage", payload) + + result = post(cordelia, dict(type="new_task_list_title", title="School Work")) + self.assert_json_success(result) + + result = post(hamlet, dict(type="new_task_list_title", title="School Work")) + self.assert_json_error( + result, "You can't edit the task list title unless you are the author." + ) + def test_poll_type_validation(self) -> None: sender = self.example_user("cordelia") stream_name = "Verona" diff --git a/zerver/views/submessage.py b/zerver/views/submessage.py index b82ab81ab6..f28bc7e236 100644 --- a/zerver/views/submessage.py +++ b/zerver/views/submessage.py @@ -49,7 +49,7 @@ def process_submessage( if widget_type == "todo": try: - validate_todo_data(todo_data=widget_data) + validate_todo_data(todo_data=widget_data, is_widget_author=is_widget_author) except ValidationError as error: raise JsonableError(error.message)