Repository: CERT-Polska/mwdb-core (language: python)

Issue:
Aggregate all object properties into ObjectContext
**Feature Category**

- [ ] Correctness
- [ ] User Interface / User Experience
- [x] Performance
- [ ] Other (please explain)

**Describe the problem**

Currently the only shared properties are the properties that are loaded by main ObjectView-level component, so other things need to be loaded by each component and can't be shared between them. E.g. if two boxes need to have access to attributes, it needs to be loaded twice. That problem may occur during plugin implementation.

**Describe the solution you'd like**

- `ObjectContext` should expose the variety of methods that make it able to update main object data, tags, attributes, comments and so on. All of them should be spawned on mount on ObjectContext.Provider level and exposed via common context property for update the specific value. Not everything is available directly from `/api/object/<id>`, so we may need to call several API endpoints for that.
- If consumer doesn't see the data loaded (e.g. `AttributeBox` see that `attributes` are missing), it should render `Loading` message inside the box
- If consumer need to update the value because something was added (e.g. via `AttributeAddModal`), it should spawn the appropriate update method from context

Common context values might be used by rich-text rendering context introduced in #602 

**Describe alternatives you've considered**

Load things at `Box` level and expose `updateObjectData` method to update the main context and share it with other components.



Discussion / hints:

Fix patch:
diff --git a/mwdb/web/src/components/ShowObject/ShowObject.js b/mwdb/web/src/components/ShowObject/ShowObject.js
index 6657b2404..bae997a20 100644
--- a/mwdb/web/src/components/ShowObject/ShowObject.js
+++ b/mwdb/web/src/components/ShowObject/ShowObject.js
@@ -27,13 +27,21 @@ const initialObjectState = {
     objectError: null,
 };
 
+const objectLoad = Symbol("objectLoad");
 const objectUpdate = Symbol("objectUpdate");
 const objectError = Symbol("objectError");
 
 function objectStateReducer(state, action) {
     switch (action.type) {
-        case objectUpdate:
+        case objectLoad:
+            // Reload object and wipe lazy-loaded updates
             return { object: action.object, objectError: null };
+        case objectUpdate:
+            // Update object data with new information
+            return {
+                object: { ...state.object, ...action.object },
+                objectError: null,
+            };
         case objectError:
             return { object: state.object, objectError: action.error };
         default:
@@ -67,20 +75,30 @@ export default function ShowObject({
         []
     );
 
-    const updateObject = useCallback(async () => {
-        try {
-            let response = await api.getObject(objectType, objectId);
-            setObjectState({
-                type: objectUpdate,
-                object: response.data,
-            });
-        } catch (error) {
-            setObjectError(error);
-        }
-    }, [api, objectType, objectId, setObjectError]);
+    const updateObject = useCallback(
+        async (doLoad) => {
+            try {
+                let response = await api.getObject(objectType, objectId);
+                setObjectState({
+                    type: doLoad ? objectLoad : objectUpdate,
+                    object: response.data,
+                });
+            } catch (error) {
+                setObjectError(error);
+            }
+        },
+        [api, objectType, objectId, setObjectError]
+    );
+
+    const updateObjectData = useCallback((objectData) => {
+        setObjectState({
+            type: objectUpdate,
+            object: objectData,
+        });
+    }, []);
 
     useEffect(() => {
-        updateObject();
+        updateObject(true);
     }, [updateObject]);
 
     const objectLayout = objectState.object ? (
@@ -134,9 +152,17 @@ export default function ShowObject({
             objectType: objectType,
             searchEndpoint: searchEndpoint,
             updateObject,
+            updateObjectData,
             setObjectError,
         }),
-        [objectState, objectType, searchEndpoint, updateObject, setObjectError]
+        [
+            objectState,
+            objectType,
+            searchEndpoint,
+            updateObject,
+            updateObjectData,
+            setObjectError,
+        ]
     );
 
     return (
diff --git a/mwdb/web/src/components/ShowObject/Views/AttributesBox.js b/mwdb/web/src/components/ShowObject/Views/AttributesBox.js
index d09569876..223e73353 100644
--- a/mwdb/web/src/components/ShowObject/Views/AttributesBox.js
+++ b/mwdb/web/src/components/ShowObject/Views/AttributesBox.js
@@ -21,10 +21,10 @@ export default function AttributesBox() {
     const [attributeIdToRemove, setAttributeIdToRemove] = useState(null);
 
     const [attributeDefinitions, setAttributeDefinitions] = useState(null);
-    const [attributes, setAttributes] = useState(null);
     const objectId = context.object.id;
-    const setObjectError = context.setObjectError;
-    const dataLoaded = attributeDefinitions !== null && attributes !== null;
+    const { setObjectError, updateObjectData } = context;
+    const attributes = context.object.attributes;
+    const dataLoaded = attributeDefinitions && attributes;
 
     async function updateAttributeDefinitions() {
         try {
@@ -45,7 +45,6 @@ export default function AttributesBox() {
     }
 
     async function updateAttributes() {
-        if (typeof objectId === "undefined") return;
         try {
             const response = await api.getObjectAttributes(objectId);
             const attributes = response.data.attributes.reduce(
@@ -60,7 +59,7 @@ export default function AttributesBox() {
                 }),
                 {}
             );
-            setAttributes(attributes);
+            updateObjectData({ attributes });
         } catch (error) {
             setObjectError(error);
         }
@@ -99,6 +98,7 @@ export default function AttributesBox() {
         objectId,
         api,
         setObjectError,
+        updateObjectData,
     ]);
 
     useEffect(() => {
diff --git a/mwdb/web/src/components/ShowObject/Views/CommentBox.js b/mwdb/web/src/components/ShowObject/Views/CommentBox.js
index f84c3d577..8a136c235 100644
--- a/mwdb/web/src/components/ShowObject/Views/CommentBox.js
+++ b/mwdb/web/src/components/ShowObject/Views/CommentBox.js
@@ -8,7 +8,6 @@ import { APIContext } from "@mwdb-web/commons/api/context";
 import { AuthContext, Capability } from "@mwdb-web/commons/auth";
 import { ObjectContext } from "@mwdb-web/commons/context";
 import { Identicon, ConfirmationModal } from "@mwdb-web/commons/ui";
-import { updateActivePage } from "@mwdb-web/commons/helpers";
 
 function Comment(props) {
     return (
@@ -45,28 +44,6 @@ function Comment(props) {
     );
 }
 
-function CommentList(props) {
-    const commentNodes = props.data.map((comment, index) => (
-        <Comment
-            id={comment.id}
-            comment={comment.comment}
-            author={comment.author}
-            timestamp={comment.timestamp}
-            removeComment={props.removeComment}
-            key={index}
-        />
-    ));
-    return (
-        <div className="commentList">
-            {commentNodes.length > 0 ? (
-                <ul className="list-group list-group-flush">{commentNodes}</ul>
-            ) : (
-                <div className="text-muted">No comments to display</div>
-            )}
-        </div>
-    );
-}
-
 function CommentForm(props) {
     const [text, setText] = useState("");
 
@@ -106,29 +83,89 @@ function CommentForm(props) {
     );
 }
 
+function CommentList({ comments, removeComment }) {
+    const itemsCountPerPage = 5;
+    const [activePage, setActivePage] = useState(1);
+
+    useEffect(() => {
+        if (
+            comments &&
+            comments.length <= itemsCountPerPage * (activePage - 1)
+        ) {
+            // Come back to first page if activePage is too big
+            setActivePage(1);
+        }
+    }, [activePage, comments]);
+
+    if (!comments) {
+        return <div className="card-body text-muted">Loading data...</div>;
+    }
+
+    if (comments.length === 0) {
+        return (
+            <div className="card-body text-muted">No comments to display</div>
+        );
+    }
+    const commentNodes = comments
+        .sort((a, b) => new Date(b.timestamp) - new Date(a.timestamp))
+        .slice(
+            (activePage - 1) * itemsCountPerPage,
+            itemsCountPerPage * activePage
+        )
+        .map((comment, index) => (
+            <Comment
+                id={comment.id}
+                comment={comment.comment}
+                author={comment.author}
+                timestamp={comment.timestamp}
+                removeComment={removeComment}
+                key={index}
+            />
+        ));
+
+    return (
+        <div className="card-body">
+            <div className="commentList">
+                <ul className="list-group list-group-flush">{commentNodes}</ul>
+            </div>
+            {comments.length > itemsCountPerPage && (
+                <Pagination
+                    activePage={activePage}
+                    itemsCountPerPage={itemsCountPerPage}
+                    totalItemsCount={comments.length}
+                    pageRangeDisplayed={5}
+                    onChange={setActivePage}
+                    itemClass="page-item"
+                    linkClass="page-link"
+                />
+            )}
+        </div>
+    );
+}
+
 function CommentBox() {
     const api = useContext(APIContext);
     const auth = useContext(AuthContext);
     const context = useContext(ObjectContext);
-    const itemsCountPerPage = 5;
 
     const canRemoveComments =
         auth.hasCapability(Capability.removingComments) && !api.remote;
     const canAddComments =
         auth.hasCapability(Capability.addingComments) && !api.remote;
 
-    const [comments, setComments] = useState([]);
-    const [activePage, setActivePage] = useState(1);
     const [isDeleteModalOpen, setDeleteModalOpen] = useState(false);
     const [commentToRemove, setCommentToRemove] = useState("");
 
     const objectId = context.object.id;
-    const setObjectError = context.setObjectError;
+    const comments = context.object.comments;
+    const { setObjectError, updateObjectData } = context;
 
     async function updateComments() {
         try {
             let response = await api.getObjectComments(objectId);
-            setComments(response.data);
+            updateObjectData({
+                comments: response.data,
+            });
         } catch (error) {
             setObjectError(error);
         }
@@ -138,7 +175,7 @@ function CommentBox() {
         if (comment) {
             try {
                 await api.addObjectComment(objectId, comment);
-                updateComments();
+                await updateComments();
             } catch (error) {
                 setObjectError(error);
             }
@@ -148,7 +185,7 @@ function CommentBox() {
     async function removeComment(comment_id) {
         try {
             await api.removeObjectComment(objectId, comment_id);
-            updateComments();
+            await updateComments();
         } catch (error) {
             setObjectError(error);
         } finally {
@@ -165,6 +202,7 @@ function CommentBox() {
         api,
         objectId,
         setObjectError,
+        updateObjectData,
     ]);
 
     useEffect(() => {
@@ -178,45 +216,15 @@ function CommentBox() {
                 onRequestClose={() => setDeleteModalOpen(false)}
                 onConfirm={() => {
                     removeComment(commentToRemove);
-                    updateActivePage(
-                        activePage,
-                        comments.length,
-                        itemsCountPerPage,
-                        setActivePage
-                    );
                 }}
                 message="Remove the comment?"
                 confirmText="Remove"
             />
             <div className="card-header">Comments</div>
-            <div className="card-body">
-                <CommentList
-                    removeComment={
-                        canRemoveComments ? handleRemoveComment : null
-                    }
-                    data={comments
-                        .sort(function (a, b) {
-                            return (
-                                new Date(b.timestamp) - new Date(a.timestamp)
-                            );
-                        })
-                        .slice(
-                            (activePage - 1) * itemsCountPerPage,
-                            itemsCountPerPage * activePage
-                        )}
-                />
-            </div>
-            {comments.length > itemsCountPerPage && (
-                <Pagination
-                    activePage={activePage}
-                    itemsCountPerPage={itemsCountPerPage}
-                    totalItemsCount={comments.length}
-                    pageRangeDisplayed={5}
-                    onChange={setActivePage}
-                    itemClass="page-item"
-                    linkClass="page-link"
-                />
-            )}
+            <CommentList
+                comments={comments}
+                removeComment={canRemoveComments ? handleRemoveComment : null}
+            />
             {canAddComments && <CommentForm submitComment={submitComment} />}
         </div>
     );
diff --git a/mwdb/web/src/components/ShowObject/Views/KartonAnalysisBox.js b/mwdb/web/src/components/ShowObject/Views/KartonAnalysisBox.js
index e9ddbde95..1bbc840a4 100644
--- a/mwdb/web/src/components/ShowObject/Views/KartonAnalysisBox.js
+++ b/mwdb/web/src/components/ShowObject/Views/KartonAnalysisBox.js
@@ -166,16 +166,47 @@ function KartonAnalysisRow({ analysis, removeAnalysis }) {
     );
 }
 
+function KartonAnalysisList({ analyses, handleRemoveAnalysis }) {
+    if (!analyses) {
+        return <div className="card-body text-muted">Loading data...</div>;
+    }
+    if (analyses.length === 0) {
+        return (
+            <div className="card-body text-muted">
+                Object was never analyzed by Karton
+            </div>
+        );
+    }
+    return (
+        <table className="table table-striped table-bordered wrap-table">
+            {analyses
+                .slice()
+                .reverse()
+                .map((analysis) => (
+                    <tr key={analysis.id}>
+                        <td>
+                            <KartonAnalysisRow
+                                analysis={analysis}
+                                removeAnalysis={handleRemoveAnalysis}
+                            />
+                        </td>
+                    </tr>
+                ))}
+        </table>
+    );
+}
+
 export default function KartonAnalysisBox() {
-    const [analyses, setAnalyses] = useState([]);
     const [isResubmitPending, setResubmitPending] = useState(false);
     const [isDeleteModalOpen, setDeleteModalOpen] = useState(false);
     const [analysisToRemove, setAnalysisToRemove] = useState(null);
 
     const api = useContext(APIContext);
     const auth = useContext(AuthContext);
-    const { object, setObjectError } = useContext(ObjectContext);
+    const { object, setObjectError, updateObjectData } =
+        useContext(ObjectContext);
     const objectId = object.id;
+    const analyses = object.analyses;
     const isReanalysisAvailable = auth.hasCapability(
         Capability.kartonReanalyze
     );
@@ -183,15 +214,19 @@ export default function KartonAnalysisBox() {
     async function updateAnalyses() {
         try {
             let response = await api.getKartonAnalysesList(objectId);
-            setAnalyses(response.data.analyses);
+            updateObjectData({
+                analyses: response.data.analyses,
+            });
         } catch (error) {
             setObjectError(error);
         }
     }
+
     const getAnalyses = useCallback(updateAnalyses, [
         api,
         objectId,
         setObjectError,
+        updateObjectData,
     ]);
 
     useEffect(() => {
@@ -209,7 +244,7 @@ export default function KartonAnalysisBox() {
         setResubmitPending(true);
         try {
             await api.resubmitKartonAnalysis(objectId);
-            updateAnalyses();
+            await updateAnalyses();
         } catch (error) {
             setObjectError(error);
         }
@@ -218,7 +253,7 @@ export default function KartonAnalysisBox() {
     async function removeAnalysis(analysisId) {
         try {
             await api.removeKartonAnalysisFromObject(objectId, analysisId);
-            updateAnalyses();
+            await updateAnalyses();
         } catch (error) {
             setObjectError(error);
         } finally {
@@ -261,29 +296,10 @@ export default function KartonAnalysisBox() {
                         Reanalyze
                     </Link>
                 </div>
-                {analyses.length ? (
-                    <table className="table table-striped table-bordered wrap-table">
-                        {analyses
-                            .slice()
-                            .reverse()
-                            .map((analysis) => (
-                                <tr key={analysis.id}>
-                                    <td>
-                                        <KartonAnalysisRow
-                                            analysis={analysis}
-                                            removeAnalysis={
-                                                handleRemoveAnalysis
-                                            }
-                                        />
-                                    </td>
-                                </tr>
-                            ))}
-                    </table>
-                ) : (
-                    <div className="card-body text-muted">
-                        Object was never analyzed by Karton
-                    </div>
-                )}
+                <KartonAnalysisList
+                    analyses={analyses}
+                    handleRemoveAnalysis={handleRemoveAnalysis}
+                />
             </div>
         </Extendable>
     );
diff --git a/mwdb/web/src/components/ShowObject/Views/SharesBox.js b/mwdb/web/src/components/ShowObject/Views/SharesBox.js
index 2fd89e172..a50fd0f23 100644
--- a/mwdb/web/src/components/ShowObject/Views/SharesBox.js
+++ b/mwdb/web/src/components/ShowObject/Views/SharesBox.js
@@ -133,23 +133,69 @@ function ShareForm(props) {
     );
 }
 
+function SharingStatusIcon({ shares }) {
+    if (!shares) return [];
+
+    // Icon showing the sharing status of the object
+    const lockIcon = shares.some((share) => share.group_name === "public")
+        ? "lock-open"
+        : "lock";
+
+    const lockTooltip = shares.some((share) => share.group_name === "public")
+        ? "Object is shared with public"
+        : "Object is not shared with public";
+
+    return (
+        <span data-toggle="tooltip" title={lockTooltip}>
+            <FontAwesomeIcon
+                icon={lockIcon}
+                pull="left"
+                size="1x"
+                style={{ color: "grey", cursor: "pointer" }}
+            />
+        </span>
+    );
+}
+
+function SharesList({ shares, groups, handleShare }) {
+    if (!shares || !groups) {
+        return <div className="card-body text-muted">Loading data...</div>;
+    }
+
+    const groupedItems = groupShares(shares);
+    return (
+        <div>
+            {groupedItems.map((sharesGroup) => (
+                <ShareGroupItem {...sharesGroup} />
+            ))}
+            {handleShare ? (
+                <ShareForm onSubmit={handleShare} groups={groups} />
+            ) : (
+                []
+            )}
+        </div>
+    );
+}
+
 function SharesBox() {
     const api = useContext(APIContext);
     const context = useContext(ObjectContext);
 
     const [groups, setGroups] = useState([]);
-    const [items, setItems] = useState([]);
     const [shareReceiver, setShareReceiver] = useState("");
     const [isModalOpen, setIsModalOpen] = useState(false);
 
     const objectId = context.object.id;
-    const setObjectError = context.setObjectError;
+    const shares = context.object.shares;
+    const { setObjectError, updateObjectData } = context;
 
     async function updateShares() {
         try {
             let response = await api.getObjectShares(objectId);
             setGroups(response.data.groups);
-            setItems(response.data.shares);
+            updateObjectData({
+                shares: response.data.shares,
+            });
         } catch (error) {
             setObjectError(error);
         }
@@ -174,23 +220,13 @@ function SharesBox() {
         api,
         objectId,
         setObjectError,
+        updateObjectData,
     ]);
 
     useEffect(() => {
         getShares();
     }, [getShares]);
 
-    const groupedItems = groupShares(items);
-
-    // Icon showing the sharing status of the object
-    const lockIcon = items.some((share) => share.group_name === "public")
-        ? "lock-open"
-        : "lock";
-
-    const lockTooltip = items.some((share) => share.group_name === "public")
-        ? "Object is shared with public"
-        : "Object is not shared with public";
-
     return (
         <div className="card card-default">
             <ConfirmationModal
@@ -205,24 +241,14 @@ function SharesBox() {
             <div className="card-header">
                 <div className="media">
                     <div className="align-self-center media-body">Shares</div>
-                    <span data-toggle="tooltip" title={lockTooltip}>
-                        <FontAwesomeIcon
-                            icon={lockIcon}
-                            pull="left"
-                            size="1x"
-                            style={{ color: "grey", cursor: "pointer" }}
-                        />
-                    </span>
+                    <SharingStatusIcon shares={shares} />
                 </div>
             </div>
-            {groupedItems.map((sharesGroup) => (
-                <ShareGroupItem {...sharesGroup} />
-            ))}
-            {!api.remote ? (
-                <ShareForm onSubmit={handleShare} groups={groups} />
-            ) : (
-                []
-            )}
+            <SharesList
+                shares={shares}
+                groups={groups}
+                handleShare={!api.remote ? handleShare : null}
+            />
         </div>
     );
 }
diff --git a/mwdb/web/src/components/ShowObject/Views/TagBox.js b/mwdb/web/src/components/ShowObject/Views/TagBox.js
index b0469f32f..6e880cec2 100644
--- a/mwdb/web/src/components/ShowObject/Views/TagBox.js
+++ b/mwdb/web/src/components/ShowObject/Views/TagBox.js
@@ -1,4 +1,4 @@
-import React, { useState, useContext, useEffect, useCallback } from "react";
+import React, { useState, useContext } from "react";
 
 import { APIContext } from "@mwdb-web/commons/api/context";
 import { AuthContext, Capability } from "@mwdb-web/commons/auth";
@@ -70,18 +70,19 @@ export default function TagBox() {
     const api = useContext(APIContext);
     const auth = useContext(AuthContext);
     const context = useContext(ObjectContext);
-    const [tags, setTags] = useState([]);
     const [modalIsOpen, setModalIsOpen] = useState(null);
     const [tagToRemove, setTagToRemove] = useState(false);
 
     const objectId = context.object.id;
-    const setObjectError = context.setObjectError;
+    const tags = context.object.tags;
+    const { setObjectError, updateObjectData } = context;
 
     async function updateTags() {
         try {
             let response = await api.getObjectTags(objectId);
-            let tags = response.data;
-            setTags(tags);
+            updateObjectData({
+                tags: response.data,
+            });
         } catch (error) {
             setObjectError(error);
         }
@@ -90,7 +91,7 @@ export default function TagBox() {
     async function handleTagSubmit(tag) {
         try {
             await api.addObjectTag(objectId, tag);
-            updateTags();
+            await updateTags();
         } catch (error) {
             setObjectError(error);
         }
@@ -99,7 +100,7 @@ export default function TagBox() {
     async function tagRemove(tag) {
         try {
             await api.removeObjectTag(objectId, tag);
-            updateTags();
+            await updateTags();
         } catch (error) {
             setObjectError(error);
         } finally {
@@ -112,12 +113,6 @@ export default function TagBox() {
         setTagToRemove(tag);
     }
 
-    const getTags = useCallback(updateTags, [api, objectId, setObjectError]);
-
-    useEffect(() => {
-        getTags();
-    }, [getTags]);
-
     return (
         <div className="card card-default">
             <ConfirmationModal


Test patch:
diff --git a/mwdb/web/src/components/ShowObject/Views/LatestConfigTab.js b/mwdb/web/src/components/ShowObject/Views/LatestConfigTab.js
index a1928efa2..d61e28a33 100644
--- a/mwdb/web/src/components/ShowObject/Views/LatestConfigTab.js
+++ b/mwdb/web/src/components/ShowObject/Views/LatestConfigTab.js
@@ -19,7 +19,7 @@ export default function LatestConfigTab(props) {
                 <div>
                     {props.label}{" "}
                     {context.object.latest_config.family ? (
-                        <span class="badge badge-danger">
+                        <span className="badge badge-danger">
                             {context.object.latest_config.family}
                         </span>
                     ) : (