-
Notifications
You must be signed in to change notification settings - Fork 3
Add lock and rename content functionality to Publisher Command Center #138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
22e9353
0cc11cc
45ce6c9
9d0cac8
3e93e62
a1719b4
8efa8c0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,7 +2,9 @@ import m from "mithril"; | |
| import { format } from "date-fns"; | ||
| import Contents from "../models/Contents"; | ||
| import Languages from "./Languages"; | ||
| import LockContentButton from "./LockContentButton"; | ||
| import DeleteModal from "./DeleteModal"; | ||
| import RenameModal from "./RenameModal"; | ||
|
|
||
| const ContentsComponent = { | ||
| error: null, | ||
|
|
@@ -43,6 +45,8 @@ const ContentsComponent = { | |
| m("th", { scope: "col" }, "Date Added"), | ||
| m("th", { scope: "col" }, ""), | ||
| m("th", { scope: "col" }, ""), | ||
| m("th", { scope: "col" }, ""), | ||
| m("th", { scope: "col" }, ""), | ||
| ]), | ||
| ), | ||
| m( | ||
|
|
@@ -72,6 +76,28 @@ const ContentsComponent = { | |
| "td", | ||
| m("button", { | ||
| class: "action-btn", | ||
| ariaLabel: `Rename ${title}`, | ||
| title: `Rename ${title}`, | ||
| "data-bs-toggle": "modal", | ||
| "data-bs-target": `#renameModal-${guid}`, | ||
| }, [ | ||
| m("i", { class: "fa-solid fa-pencil" }) | ||
| ]), | ||
| ), | ||
| m( | ||
| "td", | ||
| m(LockContentButton, { | ||
| contentId: guid, | ||
| contentTitle: title, | ||
| isLocked: content["locked"], | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh sweet we have |
||
| }), | ||
| ), | ||
| m( | ||
| "td", | ||
| m("button", { | ||
| class: "action-btn", | ||
| title: `Delete ${title}`, | ||
| ariaLabel: `Delete ${title}`, | ||
| "data-bs-toggle": "modal", | ||
| "data-bs-target": `#deleteModal-${guid}`, | ||
| }, [ | ||
|
|
@@ -83,6 +109,8 @@ const ContentsComponent = { | |
| m("a", { | ||
| class: "fa-solid fa-arrow-up-right-from-square", | ||
| href: content["content_url"], | ||
| ariaLabel: `Open ${title} (opens in new tab)`, | ||
| title: `Open ${title}`, | ||
| target: "_blank", | ||
| onclick: (e) => e.stopPropagation(), | ||
| }), | ||
|
|
@@ -91,6 +119,10 @@ const ContentsComponent = { | |
| contentId: guid, | ||
| contentTitle: title, | ||
| }), | ||
| m(RenameModal, { | ||
| contentId: guid, | ||
| contentTitle: title, | ||
| }), | ||
| ], | ||
| ); | ||
| }), | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import m from "mithril"; | ||
| import Contents from "../models/Contents"; | ||
|
|
||
| const LockedContentButton = { | ||
| oninit: function () { | ||
| this.isLoading = false; | ||
| }, | ||
|
|
||
| view: function(vnode) { | ||
| const labelMessage = vnode.attrs.isLocked ? | ||
| `Unlock ${vnode.attrs.contentTitle}` : | ||
| `Lock Content ${vnode.attrs.contentTitle}`; | ||
|
|
||
| const iconClassName = () => { | ||
| if (this.isLoading) return "fa-spinner fa-spin lock-loading"; | ||
|
|
||
| if (vnode.attrs.isLocked) { | ||
| return "fa-lock"; | ||
| } else { | ||
| return "fa-lock-open"; | ||
| } | ||
| }; | ||
|
|
||
| return m("button", { | ||
| class: "action-btn", | ||
| ariaLabel: labelMessage, | ||
| title: labelMessage, | ||
| disabled: this.isLoading, | ||
| onclick: async () => { | ||
| if (this.isLoading) { return; } | ||
|
|
||
| this.isLoading = true; | ||
| m.redraw(); | ||
|
|
||
| try { | ||
| await Contents.lock(vnode.attrs.contentId) | ||
| } finally { | ||
| this.isLoading = false; | ||
| m.redraw(); | ||
| } | ||
| } | ||
| }, [ | ||
| m("i", { | ||
| class: `fa-solid ${iconClassName()}`, | ||
|
|
||
| } | ||
| ) | ||
| ]) | ||
| } | ||
| }; | ||
|
|
||
| export default LockedContentButton; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| import m from "mithril"; | ||
| import Contents from "../models/Contents"; | ||
| import { Modal } from "bootstrap"; | ||
|
|
||
| const RenameModalForm = { | ||
| newName: "", | ||
| guid: "", | ||
| isValid: false, | ||
| onsubmit: function(e) { | ||
| if (RenameModalForm.isValid) { | ||
| e.preventDefault(); | ||
| Contents.rename(RenameModalForm.guid, RenameModalForm.newName); | ||
|
|
||
| const modalEl = document.getElementById(`renameModal-${RenameModalForm.guid}`); | ||
| const modal = Modal.getInstance(modalEl); | ||
| modal.hide(); | ||
|
|
||
| RenameModalForm.newName = ""; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! |
||
| } | ||
| }, | ||
| view: function(vnode) { | ||
| return m("form", { | ||
| onsubmit: (e) => { this.onsubmit(e); } | ||
| }, | ||
| [ | ||
| m("section", { class: "modal-body" }, [ | ||
| m("div", { class: "form-group" }, [ | ||
| m("label", { | ||
| for: "rename-content-input", | ||
| class: "mb-3", | ||
| }, "Enter new name for ", [ | ||
| m("span", { class: "fw-bold" }, `${vnode.attrs.contentTitle}`) | ||
| ]), | ||
| m("input", { | ||
| oninput: function(e) { | ||
| RenameModalForm.isValid = e.target.validity.valid; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| RenameModalForm.guid = vnode.attrs.contentId; | ||
| RenameModalForm.newName = e.target.value; | ||
| }, | ||
| id: "rename-content-input", | ||
| type: "text", | ||
| class: "form-control", | ||
| required: true, | ||
| minlength: 3, | ||
| maxlength: 1024, | ||
| value: RenameModalForm.newName, | ||
| }), | ||
| ]) | ||
| ]), | ||
| m("div", { class: "modal-footer" }, [ | ||
| m( | ||
| "button", | ||
| { | ||
| type: "submit", | ||
| class: "btn btn-primary", | ||
| ariaLabel: "Rename Content", | ||
| }, | ||
| "Rename Content", | ||
| ), | ||
| ]), | ||
| ]) | ||
| }, | ||
| } | ||
|
|
||
| const RenameModal = { | ||
| view: function(vnode) { | ||
| return m("div", { | ||
| class: "modal", | ||
| id: `renameModal-${vnode.attrs.contentId}`, | ||
| tabindex: "-1", | ||
| ariaHidden: true | ||
| }, [ | ||
| m("div", { class: "modal-dialog modal-dialog-centered" }, [ | ||
| m("div", { class: "modal-content" }, [ | ||
| m("div", { class: "modal-header"}, [ | ||
| m("h1", { class: "modal-title fs-6" }, "Rename Content"), | ||
| m("button", { | ||
| class: "btn-close", | ||
| ariaLabel: "Close modal", | ||
| "data-bs-dismiss": "modal" | ||
| }), | ||
| ]), | ||
| m(RenameModalForm, { | ||
| contentTitle: vnode.attrs.contentTitle, | ||
| contentId: vnode.attrs.contentId, | ||
| }) | ||
| ]), | ||
| ]), | ||
| ]) | ||
| }, | ||
| }; | ||
|
|
||
| export default RenameModal; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,29 @@ export default { | |
| this.data = this.data.filter((c) => c.guid !== guid); | ||
| }, | ||
|
|
||
| lock: async function (guid) { | ||
| await m.request({ | ||
| method: "PATCH", | ||
| url: `api/content/${guid}/lock`, | ||
| }).then((response) => { | ||
| const targetContent = this.data.find((c) => c.guid === guid); | ||
| Object.assign(targetContent, response); | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since Mihtril.JS doesn't have reactivity like Vue or React, we need to change the content of the data in order to reflect the new changes to the UI via the automatic
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good call adjusting the data so we don't need the slower API call. |
||
| }); | ||
| }, | ||
|
|
||
| rename: async function (guid, newName) { | ||
| await m.request({ | ||
| method: "PATCH", | ||
| url: `api/content/${guid}/rename`, | ||
| body: { | ||
| title: newName, | ||
| }, | ||
| }).then((response) => { | ||
| const targetContent = this.data.find((c) => c.guid === guid); | ||
| Object.assign(targetContent, response); | ||
| }); | ||
| }, | ||
|
|
||
| reset: function () { | ||
| this.data = null; | ||
| this._fetch = null; | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My first thought with both of these was "do we already have the
contentdata?". If we do we could avoid an API call here.It looks like there isn't a good way, given only a GUID, to call
content.update(title = title)from the SDK though, perhaps that is something we could request to make things like this a bit easier so this could be faster.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually there is!
We can do:
with the additional imports:
We can make a
Contextfor theContentItemand pass theguid. SinceContentItemonly needs aContextandguidand all other attributes are optional this is enough to do the rename.The lock has
is_locked = content.lockedwhich relies on the Content'slockedattribute. If we can pass that in the FastAPI/api/content/{content_id}/lockAPI (or create two one for unlock and one for lock) then we can do the same there speeding it up by 2 times.I don't think any of this is necessary in this PR to be fair. If it is very easy we can get it in otherwise we can follow-up since it would be good deal faster.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that looks good @dotNomad! For what it's worth, I wouldn't worry about the additional API call to retrieve the content in order to update it. It's all happening across the local network, so the overhead is fairly negligible.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that is a good point - it will be much faster than local dev 👍