[PATCH v3 4/5] static: add rest.js to handle requests & respective messages

Daniel Axtens dja at axtens.net
Fri Aug 6 13:03:25 AEST 2021


Raxel Gutierrez <raxel at google.com> writes:

> Add js file to have REST API requests utilities JavaScript module to be
> reused by other Patchwork js files making updates to objects.
>
>  - Add function to make fetch requests that update properties through
>    'PATCH' requests with the REST API endpoints.
>
>  - Add functions that handle update & error messages for these 'PATCH'
>    update requests following the Django messages framework format and
>    form error styling.
>
> The subsequent patch will make use of these functions which will be also
> reused in future features the make use fetch requests to update object
> fields.

Excellent. Using the rest API from the web interface will help drag us
into only being several years behind the modern web rather than over a
decade :P

>
> Signed-off-by: Raxel Gutierrez <raxel at google.com>
> ---
>  htdocs/README.rst |  7 +++++
>  htdocs/js/rest.js | 72 +++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 79 insertions(+)
>  create mode 100644 htdocs/js/rest.js
>
> diff --git a/htdocs/README.rst b/htdocs/README.rst
> index 404356a..2e76013 100644
> --- a/htdocs/README.rst
> +++ b/htdocs/README.rst
> @@ -138,6 +138,13 @@ js
>  
>    Part of Patchwork.
>  
> +``rest.js.``
> +
> +  Utility module for REST API requests to be used by other Patchwork js files
> +  (fetch requests, handling update & error messages).
> +
> +  Part of Patchwork.
> +
>  ``selectize.min.js``
>  
>    Selectize is the hybrid of a ``textbox`` and ``<select>`` box. It's jQuery
> diff --git a/htdocs/js/rest.js b/htdocs/js/rest.js
> new file mode 100644
> index 0000000..18c0295
> --- /dev/null
> +++ b/htdocs/js/rest.js
> @@ -0,0 +1,72 @@
> +/**
> + * Sends fetch requests to update objects' properties using REST api endpoints.
> + * @param {string} url Path to the REST api endpoint.
> + * @param {{field: string, value: string}} data
> + *     field: Name of the property field to update.
> + *     value: Value to update the property field to.
> + * @param {{none: string, some: string}} updateMessage
> + *     none: Message when object update unsuccessful due to errors.
> + *     some: Message when object update successful.
> + */
> +async function updateProperty(url, data, updateMessage) {
> +    const request = new Request(url, {
> +        method: 'PATCH',
> +        mode: "same-origin",
> +        headers: {
> +            "X-CSRFToken": Cookies.get("csrftoken"),
> +            "Content-Type": "application/json",
> +        },
> +        body: JSON.stringify(data),
> +    });
> +
> +    return await fetch(request)
> +        .then(response => {
> +            if (!response.ok) {
> +                response.text().then(text => {
> +                    // Error occurred, so update message specifies no objects updated
> +                    handleUpdateMessages(updateMessage.none);
> +                    handleErrorMessages(JSON.parse(text).detail);
> +                });
> +            } else {
> +                // Update message for successful changes
> +                handleUpdateMessages(updateMessage.some);
> +            }
> +            return response.ok
> +        });
> +}
> +
> +/**
> + * Populates update messages for fetch requests.
> + * @param {string} messageContent Text for update message.
> + */
> +function handleUpdateMessages(messageContent) {
> +    let messages = document.getElementById("messages");
> +    if (messages == null) {
> +        messages = document.createElement("div");
> +        messages.setAttribute("id", "messages");
> +    }
> +    let message = document.createElement("div");
> +    message.setAttribute("class", "message");
> +    message.textContent = messageContent;
> +    messages.appendChild(message);
> +    if (messages) $(messages).insertAfter("nav");
> +}
> +
> +/**
> + * Populates error messages for fetch requests.
> + * @param {string} errorMessage Text for error message.
> + */
> +function handleErrorMessages(errorMessage) {
> +    let container = document.getElementById("main-content");
> +    let errorHeader = document.createElement("p");
> +    let errorList = document.createElement("ul");
> +    let error = document.createElement("li");
> +    errorHeader.textContent = "The following error was encountered while updating comments:";
> +    errorList.setAttribute("class", "errorlist");
> +    error.textContent = errorMessage;
> +    errorList.appendChild(error);
> +    container.prepend(errorList);
> +    container.prepend(errorHeader);
> +}
> +
> +export { updateProperty, handleUpdateMessages, handleUpdateMessages};

Chrome doesn't like the double-up of handleUpdateMessages here, it
throws an error on the console.

I haven't reviewed the rest of the code and I'm not sure I'm super
qualified to do so. I guess I can give it a go though at some point.

One thing to consider: currently the messages never time out. So if I
mark 3 comments as addressed/unaddressed (using the example from the
other series) I end up with a top message bar that reads:

1 comment updated
1 comment updated
1 comment updated

You don't need to make it smart enough to read "3 comments updated"! but
maybe we need to figure out if the list should only keep M messages or
if the list should clear after N seconds.

> \ No newline at end of file

Please add a newline at the end of files. (This is a completely
aesthetic preference of mine, I just don't like the git diff noise.)

> -- 
> 2.32.0.554.ge1b32706d8-goog
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork


More information about the Patchwork mailing list