feat(Topic Editor): update props that change

reader/views.py

@@ -3112,7 +3112,8 @@ def add_new_topic_api(request):
 
         t.description_published = True
         t.data_source = "sefaria"  # any topic edited manually should display automatically in the TOC and this flag ensures this
-        t.change_description(data["description"], data.get("catDescription", None))
+        if "description" in data:
+            t.change_description(data["description"], data.get("categoryDescription", None))
         t.save()
 
         library.build_topic_auto_completer()
@@ -3165,8 +3166,6 @@ def topics_api(request, topic, v2=False):
         topic_obj = Topic().load({'slug': topic_data["origSlug"]})
         topic_data["manual"] = True
         author_status_changed = (topic_data["category"] == "authors") ^ (topic_data["origCategory"] == "authors")
-        if topic_data["category"] == topic_data["origCategory"]:
-            topic_data.pop("category")  # no need to check category in update_topic
         topic_obj = update_topic(topic_obj, **topic_data)
         if author_status_changed:
             library.build_topic_auto_completer()

sefaria/helper/topic.py

@@ -1060,35 +1060,43 @@ def update_authors_place_and_time(topic_obj, data, dataSource='learning-team-edi
 
     return topic_obj
 
+def update_properties(topic_obj, dataSource, k, v):
+    if v == '':
+        topic_obj.properties.pop(k, None)
+    else:
+        topic_obj.properties[k] = {'value': v, 'dataSource': dataSource}
+
 def update_author_era(topic_obj, data, dataSource='learning-team-editing-tool'):
     for k in ["birthYear", "deathYear"]:
-        year = data.get(k, '')
-        topic_obj.properties[k] = {'value': year, 'dataSource': dataSource}
-
-    prev_era = topic_obj.properties.get('era', {}).get('value')
-    if data.get('era', '') != '' and prev_era != data['era']:
-        topic_obj.properties['era'] = {'value': data['era'], 'dataSource': dataSource}
-        create_era_link(topic_obj, prev_era_to_delete=prev_era)
+        if k in data.keys():    # only change property value if key is in data, otherwise it indicates no change
+            year = data[k]
+            update_properties(topic_obj, dataSource, k, year)
+
+    if 'era' in data.keys():    # only change property value if key is in data, otherwise it indicates no change
+        prev_era = topic_obj.properties.get('era', {}).get('value')
+        era = data['era']
+        update_properties(topic_obj, dataSource, 'era', era)
+        if era != '':
+            create_era_link(topic_obj, prev_era_to_delete=prev_era)
     return topic_obj
 
 
 def update_topic(topic_obj, **kwargs):
     """
     Can update topic object's title, hebrew title, category, description, and categoryDescription fields
     :param topic_obj: (Topic) The topic to update
-    :param **kwargs can be title, heTitle, category, description, catDescription, and rebuild_toc where `title`, `heTitle`,
-         and `category` are strings. `description` and `catDescription` are dictionaries where the fields are `en` and `he`.
+    :param **kwargs can be title, heTitle, category, description, categoryDescription, and rebuild_toc where `title`, `heTitle`,
+         and `category` are strings. `description` and `categoryDescription` are dictionaries where the fields are `en` and `he`.
          The `category` parameter should be the slug of the new category. `rebuild_topic_toc` is a boolean and is assumed to be True
     :return: (model.Topic) The modified topic
     """
     old_category = ""
     orig_slug = topic_obj.slug
-
     update_topic_titles(topic_obj, kwargs)
-    if kwargs.get('catSlug') == 'authors':
+    if kwargs.get('category') == 'authors':
         topic_obj = update_authors_place_and_time(topic_obj, kwargs)
 
-    if 'category' in kwargs:
+    if 'category' in kwargs and kwargs['category'] != kwargs['origCategory']:
         orig_link = IntraTopicLink().load({"linkType": "displays-under", "fromTopic": topic_obj.slug, "toTopic": {"$ne": topic_obj.slug}})
         old_category = orig_link.toTopic if orig_link else Topic.ROOT
         if old_category != kwargs['category']:
@@ -1098,8 +1106,8 @@ def update_topic(topic_obj, **kwargs):
         topic_obj.data_source = "sefaria"  # any topic edited manually should display automatically in the TOC and this flag ensures this
         topic_obj.description_published = True
 
-    if "description" in kwargs:
-        topic_obj.change_description(kwargs["description"], kwargs.get("catDescription", None))
+    if "description" in kwargs or "categoryDescription" in kwargs:
+        topic_obj.change_description(kwargs.get("description", None), kwargs.get("categoryDescription", None))
 
     topic_obj.save()
 

sefaria/model/place.py

@@ -108,12 +108,13 @@ def process_index_place_change(indx, **kwargs):
 def process_topic_place_change(topic_obj, data):
     keys = ["birthPlace", "deathPlace"]
     for key in keys:
-        he_key = get_he_key(key)
-        he_new_val = data.get(he_key, '')
-        new_val = data.get(key, '')
-        if new_val != '':
-            place = Place.create_new_place(en=new_val, he=he_new_val)
-            topic_obj.properties[key] = {"value": place.primary_name('en'), 'dataSource': 'learning-team-editing-tool'}
-        else:
-            topic_obj.properties.pop(key, None)
+        if key in data.keys():  # only change property value if key is in data, otherwise it indicates no change
+            new_val = data[key]
+            if new_val != '':
+                he_key = get_he_key(key)
+                he_new_val = data.get(he_key, '')
+                place = Place.create_new_place(en=new_val, he=he_new_val)
+                topic_obj.properties[key] = {"value": place.primary_name('en'), 'dataSource': 'learning-team-editing-tool'}
+            else:
+                topic_obj.properties.pop(key, None)
 

sefaria/model/topic.py

@@ -136,10 +136,13 @@ def change_description(self, desc, cat_desc=None):
         :param cat_desc: Optional. Dictionary of category descriptions, with keys being two letter language codes
         :return:
         """
-
+        if cat_desc is None:
+            cat_desc = {"en": "", "he": ""}
+        if desc is None:
+            desc = {"en": "", "he": ""}
         self.description = desc
         if getattr(self, "isTopLevelDisplay", False):
-            self.categoryDescription = cat_desc if cat_desc else {"en": "", "he": ""}
+            self.categoryDescription = cat_desc
         elif getattr(self, "categoryDescription", False):
             delattr(self, "categoryDescription")
 

static/js/Misc.jsx

@@ -1191,11 +1191,13 @@ const EditorForExistingTopic = ({ toggle, data }) => {
   const initCatSlug = TopicToCategorySlug(data);
   const origData = {
     origSlug: data.slug,
-    origCategorySlug: initCatSlug,
-    origEn: data.primaryTitle.en,
-    origHe: data.primaryTitle.he || "",
-    origDesc: data.description || {"en": "", "he": ""},
-    origCategoryDesc: data.categoryDescription || {"en": "", "he": ""},
+    origCatSlug: initCatSlug,
+    origEnTitle: data.primaryTitle.en,
+    origHeTitle: data.primaryTitle.he || "",
+    origEnDescription: data.description?.en || "",
+    origHeDescription: data.description?.he || "",
+    origEnCategoryDescription: data.categoryDescription?.en || "",
+    origHeCategoryDescription: data.categoryDescription?.he || "",
     origEnAltTitles: prepAltTitles('en'),
     origHeAltTitles: prepAltTitles('he'),
     origBirthPlace: data?.properties?.birthPlace?.value,
@@ -1257,12 +1259,12 @@ const CategoryEditorWrapper = ({toggle, data, type}) => {
 }
 
 const CategoryAdderWrapper = ({toggle, data, type}) => {
-      const origData = {origEn: ""};
+      const origData = {origEnTitle: ""};
       switch (type) {
         case "cats":
           return <CategoryEditor origData={origData} close={toggle} origPath={data}/>;
         case "topics":
-          origData['origCategorySlug'] = data;
+          origData['origCatSlug'] = data;
           return <TopicEditor origData={origData} close={toggle} origWasCat={false}/>;
       }
   }

static/js/TopicEditor.jsx

@@ -7,11 +7,12 @@ import React, {useState, useEffect} from "react";
 
 
 const TopicEditor = ({origData, onCreateSuccess, close, origWasCat}) => {
-    const [data, setData] = useState({...origData, catSlug: origData.origCategorySlug || "",
-                                enTitle: origData.origEn, heTitle: origData.origHe || "", heDescription: origData?.origDesc?.he || "",
-                                enDescription: origData?.origDesc?.en || "",
-                                enCategoryDescription: origData?.origCategoryDesc?.en,
-                                heCategoryDescription: origData?.origCategoryDesc?.he,
+    const [data, setData] = useState({...origData, catSlug: origData.origCatSlug || "",
+                                enTitle: origData.origEnTitle, heTitle: origData.origHeTitle || "",
+                                heDescription: origData?.origHeDescription || "",
+                                enDescription: origData?.origEnDescription || "",
+                                enCategoryDescription: origData?.origEnCategoryDescription || "",
+                                heCategoryDescription: origData?.origHeCategoryDescription || "",
                                 enAltTitles: origData?.origEnAltTitles || [],
                                 heAltTitles: origData?.origHeAltTitles || [],
                                 birthPlace: origData.origBirthPlace || "", heBirthPlace: origData.origHeBirthPlace || "",
@@ -21,7 +22,7 @@ const TopicEditor = ({origData, onCreateSuccess, close, origWasCat}) => {
                                 });
     const isNew = !('origSlug' in origData);
     const [savingStatus, setSavingStatus] = useState(false);
-    const [isAuthor, setIsAuthor] = useState(origData.origCategorySlug === 'authors');
+    const [isAuthor, setIsAuthor] = useState(origData.origCatSlug === 'authors');
     const [isCategory, setIsCategory] = useState(origWasCat);  // initialize to True if the topic originally was a category
                                                                   // isCategory determines whether user can edit categoryDescriptions of topic
     const subtopics = Sefaria.topicTocPage(origData.origSlug);
@@ -40,6 +41,7 @@ const TopicEditor = ({origData, onCreateSuccess, close, origWasCat}) => {
       //logic is: if it starts out originally a category, isCategory should always be true, otherwise, it should depend solely on 'Main Menu'
       const newIsCategory = origWasCat || e.target.value === Sefaria._("Main Menu");
       setIsCategory(newIsCategory);
+      setIsChanged(true);
       setIsAuthor(data.catSlug === 'authors');
       setData({...data});
     }
@@ -92,25 +94,39 @@ const TopicEditor = ({origData, onCreateSuccess, close, origWasCat}) => {
     }
 
     const prepData = () => {
-        let postData = {...data, "description": {"en": data.enDescription, "he": data.heDescription}, "title": data.enTitle,
-            "heTitle": data.heTitle};
-        if (isCategory) {
-            postData = {...postData, "catDescription": {"en": data.enCategoryDescription, "he": data.heCategoryDescription}};
+        // always add category, title, heTitle, altTitles
+        let postData = { category: data.catSlug, title: data.enTitle, heTitle: data.heTitle, altTitles: {}};
+        postData.altTitles.en = data.enAltTitles.map(x => x.name); // alt titles implemented using TitleVariants which contains list of objects with 'name' property.
+        postData.altTitles.he = data.heAltTitles.map(x => x.name);
+
+        // add descriptions if they changed
+        const origDescription = {en: origData?.origEnDescription || "", he: origData?.origHeDescription || ""};
+        const origCategoryDescription = {en: origData?.origEnCategoryDescription || "", he: origData?.origHeCategoryDescription || ""};
+        const descriptionChanged = data.enDescription !== origDescription.en || data.heDescription !== origDescription.he;
+        if (descriptionChanged) {
+            postData.description = {en: data.enDescription, he: data.heDescription};
         }
-        if (data?.era && Sefaria.util.inArray(data.era, Sefaria._eras) === -1) {
-            delete postData.era;
+        const categoryDescriptionChanged = data.enCategoryDescription !== origCategoryDescription.en || data.heCategoryDescription !== origCategoryDescription.he
+        if (isCategory && categoryDescriptionChanged) {
+            postData.categoryDescription = {en: data.enCategoryDescription, he: data.heCategoryDescription};
         }
-        postData.altTitles = {};
-         // alt titles implemented using TitleVariants which contains list of objects with 'name' property.
-        postData.altTitles.en = postData.enAltTitles.map(x => x.name);
-        postData.altTitles.he = postData.heAltTitles.map(x => x.name);
-        postData.category = data.catSlug;
+
+        // add author keys if they changed
+        if (isAuthor) {
+            let authorKeys = ['era', 'birthPlace', 'heBirthPlace', 'birthYear', 'deathYear', 'deathPlace', 'heDeathPlace'];
+            authorKeys.map(k => {
+                const firstLetter = k.charAt(0).toUpperCase();
+                const origKey = `orig${firstLetter}${k.slice(1)}`;
+                const origVal = origData[origKey] || "";
+                const newVal = data[k] || "";
+                if (origVal !== newVal) {
+                    postData[k] = data[k];
+                }
+            })
+        }
+
         if (!isNew) {
-          postData = {...postData, origCategory: data.origCategorySlug, origDescription: data.origDesc,
-                    origSlug: data.origSlug};
-          if (isCategory) {
-            postData.origCatDescription = data.origCategoryDesc;
-          }
+          postData = {...postData, origSlug: data.origSlug, origCategory: data.origCatSlug};
         }
 
         return postData;

static/js/TopicSearch.jsx

@@ -108,7 +108,7 @@ class TopicSearch extends Component {
 
   render() {
         if (this.state.showAdminEditor) {
-            const topicData = {origEn: this.state.value};
+            const topicData = {origEnTitle: this.state.value};
             return <TopicEditor origData={topicData} close={this.reset} onCreateSuccess={this.post}/>;
         }
         else {