Message ID | 20210719233428.2045872-5-raxel@google.com |
---|---|
State | Superseded |
Headers | show |
Series | patch-list: improve usability of list action bar | expand |
Hi, Raxel Gutierrez wrote: > Added file to have a general utilities JavaScript module file for > functions that are reused throughout various Patchwork js files. First > added function for making fetch requests to update properties through > 'PATCH' requests with the REST api endpoints. Also, added functions for > handling update & error messages for these requests. 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. > > Signed-off-by: Raxel Gutierrez <raxel@google.com> > --- > htdocs/README.rst | 7 +++++ > htdocs/js/utils.js | 71 ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 78 insertions(+) > create mode 100644 htdocs/js/utils.js I think I'd prefer to see this squashed into the same patch as patch 5/5, because that way it's easier to evaluate the interface it exposes in the context of what its callers need. I don't feel strongly about that, though. (It mostly comes down to whether the API exposed here is obvious enough when viewed without that context.) > diff --git a/htdocs/README.rst b/htdocs/README.rst > index 4441bf3..2bae34c 100644 > --- a/htdocs/README.rst > +++ b/htdocs/README.rst > @@ -147,3 +147,10 @@ js > :Website: https://selectize.github.io/selectize.js/ > :GitHub: https://github.com/selectize/selectize.js > :Version: 0.11.2 > + > +``utils.js.`` > + > + General utility module for functions used throughout other static Patchwork > + js files (fetch requests, handling update & error messages). > + > + Part of Patchwork. These appear to be specifically about making REST API requests to the patchwork backend, so how about a name like rest.js? That way, if we come up with other utility functions then we can group them into their own utility library. Thanks, Jonathan
Hi, > I think I'd prefer to see this squashed into the same patch as patch > 5/5, because that way it's easier to evaluate the interface it exposes > in the context of what its callers need. I don't feel strongly about > that, though. (It mostly comes down to whether the API exposed here > is obvious enough when viewed without that context.) I feel that the file is well documented enough and the methods it exposes are also given enough context for the patch to be on its own. Patch 5/5 adds a whole new feature to Patchwork which I feel should be evaluated by itself. Without squashing, both patches can be more self-contained. Ultimately, I feel that the following patch is more than just a proof of concept, it's a new feature. This patch simply is for healthier code as the module's functions will be used elsewhere later.
diff --git a/htdocs/README.rst b/htdocs/README.rst index 4441bf3..2bae34c 100644 --- a/htdocs/README.rst +++ b/htdocs/README.rst @@ -147,3 +147,10 @@ js :Website: https://selectize.github.io/selectize.js/ :GitHub: https://github.com/selectize/selectize.js :Version: 0.11.2 + +``utils.js.`` + + General utility module for functions used throughout other static Patchwork + js files (fetch requests, handling update & error messages). + + Part of Patchwork. diff --git a/htdocs/js/utils.js b/htdocs/js/utils.js new file mode 100644 index 0000000..34d41f5 --- /dev/null +++ b/htdocs/js/utils.js @@ -0,0 +1,71 @@ +/** + * 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), + }); + + 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); + } + }); +} + +/** + * 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}; \ No newline at end of file
Added file to have a general utilities JavaScript module file for functions that are reused throughout various Patchwork js files. First added function for making fetch requests to update properties through 'PATCH' requests with the REST api endpoints. Also, added functions for handling update & error messages for these requests. 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. Signed-off-by: Raxel Gutierrez <raxel@google.com> --- htdocs/README.rst | 7 +++++ htdocs/js/utils.js | 71 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) create mode 100644 htdocs/js/utils.js