From cc3b28abd7f997e0bcafdb0f7158fe60b04a3c16 Mon Sep 17 00:00:00 2001
From: raghu <http://localhost:9000>
Date: Fri, 23 Nov 2018 14:20:49 +0530
Subject: [PATCH] Issue #SB-8572 fix : code review rework

---
 src/config/constants.json     |  4 +-
 src/helpers/orgHelper.js      | 76 ++++++++++++++++++++++-------------
 src/service/contentService.js | 28 ++-----------
 src/service/courseService.js  | 37 ++---------------
 4 files changed, 57 insertions(+), 88 deletions(-)

diff --git a/src/config/constants.json b/src/config/constants.json
index e299e6f..e50dbcd 100644
--- a/src/config/constants.json
+++ b/src/config/constants.json
@@ -1,5 +1,5 @@
 {
     "orgCacheKeyName": "allRootOrgs",
-    "orgCacheExpiryTime": "",
-    "orgfieldsAllowed": ["email","orgName"]
+    "orgCacheExpiryTime": 1800,
+    "orgfieldsAllowedToSend": ["email","orgName"]
 }   
\ No newline at end of file
diff --git a/src/helpers/orgHelper.js b/src/helpers/orgHelper.js
index 137a963..23b5421 100644
--- a/src/helpers/orgHelper.js
+++ b/src/helpers/orgHelper.js
@@ -44,16 +44,20 @@ function getRootOrgsFromCache (CBW) {
         if (err) {
           CBW(err)
         } else {
-          cacheManager.set({ key: configData.orgCacheKeyName, value: res.result.response.content },
-            function (err, data) {
-              if (err) {
-                LOG.error(utilsService.getLoggerData({}, 'ERROR', filename, 'Setting allRootOrgs cache failed',
-                  'Setting allRootOrgs cache data failed', err))
-              } else {
-                LOG.info(utilsService.getLoggerData({}, 'INFO', filename,
-                  'Setting allRootOrgs cache data success'))
-              }
-            })
+          var cacheinputdata = {
+            key: configData.orgCacheKeyName,
+            value: res.result.response.content,
+            ttl: configData.orgCacheExpiryTime
+          }
+          cacheManager.set(cacheinputdata, function (err, data) {
+            if (err) {
+              LOG.error(utilsService.getLoggerData({}, 'ERROR', filename, 'Setting allRootOrgs cache failed',
+                'Setting allRootOrgs cache data failed', err))
+            } else {
+              LOG.info(utilsService.getLoggerData({}, 'INFO', filename,
+                'Setting allRootOrgs cache data success'))
+            }
+          })
           CBW(null, res.result.response.content)
         }
       })
@@ -70,32 +74,50 @@ function getRootOrgsFromCache (CBW) {
  */
 function populateOrgDetailsByHasTag (inputdata, inputfields, cb) {
   inputfields = inputfields.split(',')
-  var fieldsToPopulate = configData.orgfieldsAllowed.filter(eachfield => inputfields.includes(eachfield))
-  if (fieldsToPopulate && fieldsToPopulate.length && inputdata.length) {
+  var fieldsToPopulate = configData.orgfieldsAllowedToSend.filter(eachfield => inputfields.includes(eachfield))
+  if (_.size(fieldsToPopulate) > 0 && _.size(inputdata) > 0) {
     getRootOrgsFromCache(function (err, orgdata) {
+      if (!err && orgdata) {
+        var orgDetails = _.keyBy(orgdata, 'hashTagId')
+        _.forEach(inputdata, (eachcontent, index) => {
+          if (eachcontent.channel) {
+            var eachorgdetail = orgDetails[eachcontent.channel]
+            inputdata[index].orgDetails = eachorgdetail ? _.pick(eachorgdetail, fieldsToPopulate) : {}
+          }
+        })
+      };
+      cb(null, inputdata)
+    })
+  } else {
+    cb(null, inputdata)
+  }
+}
+
+/**
+ * This function loops each object from the input and includes org details in it
+ * @param inputdata is req object and res object
+ * @param cb there will be no error callback , always returns success
+ */
+function includeOrgDetails (req, res, cb) {
+  if (_.get(req, 'query.orgdetails') && _.get(res, 'result.content')) {
+    var fields = req.query.orgdetails
+    var inputContentIsArray = _.isArray(res.result.content)
+    // res.result.content need to send as array bec populateOrgDetailsByHasTag expects data as array
+    res.result.content = inputContentIsArray ? res.result.content : [res.result.content]
+    populateOrgDetailsByHasTag(res.result.content, fields, function
+      (err, contentwithorgdetails) {
       if (!err) {
-        if (orgdata) {
-          var orgDetails = _.keyBy(orgdata, 'hashTagId')
-          _.forEach(inputdata, (eachcontent, index) => {
-            if (eachcontent.channel) {
-              var eachorgdetail = orgDetails['01258430492140339266']
-              if (eachorgdetail) {
-                inputdata[index].orgDetails = _.pick(eachorgdetail, fieldsToPopulate)
-              }
-            }
-          })
-        }
-        cb(null, inputdata)
-      } else {
-        cb(null, inputdata)
+        res.result.content = inputContentIsArray ? contentwithorgdetails : contentwithorgdetails[0]
       }
+      cb(null, res)
     })
   } else {
-    cb(null, inputdata)
+    cb(null, res)
   }
 }
 
 module.exports = {
   getRootOrgs: getRootOrgs,
+  includeOrgDetails: includeOrgDetails,
   populateOrgDetailsByHasTag: populateOrgDetailsByHasTag
 }
diff --git a/src/service/contentService.js b/src/service/contentService.js
index c97126a..5649d25 100644
--- a/src/service/contentService.js
+++ b/src/service/contentService.js
@@ -131,22 +131,11 @@ function search (defaultContentTypes, req, response, objectType) {
                 lodash.get(data, 'result.framework.categories')) {
                   modifyFacetsData(res.result.facets, data.result.framework.categories, language)
                 }
-                if (req.query && req.query.orgdetails && res.result && res.result.content) {
-                  var fields = req.query.orgdetails
-                  orgHelper.populateOrgDetailsByHasTag(res.result.content, fields, function
-                    (err, contentwithorgdetails) {
-                    if (!err) {
-                      res.result.content = contentwithorgdetails
-                    }
-                    CBW(null, res)
-                  })
-                } else {
-                  CBW(null, res)
-                }
+                orgHelper.includeOrgDetails(req, res, CBW)
               }
             })
           } else {
-            CBW(null, res)
+            orgHelper.includeOrgDetails(req, res, CBW)
           }
         }
       })
@@ -637,18 +626,7 @@ function getContentAPI (req, response) {
       })
     },
     function (res, CBW) {
-      if (req.query && req.query.orgdetails && res.result && res.result.content) {
-        var fields = req.query.orgdetails
-        // sending res.result.content as array bec populateOrgDetailsByHasTag expects data as array
-        orgHelper.populateOrgDetailsByHasTag([res.result.content], fields, function (err, courseswithorgdetails) {
-          if (!err) {
-            res.result.content = courseswithorgdetails[0]
-          }
-          CBW(null, res)
-        })
-      } else {
-        CBW(null, res)
-      };
+      orgHelper.includeOrgDetails(req, res, CBW)
     },
     function (res) {
       rspObj.result = res.result
diff --git a/src/service/courseService.js b/src/service/courseService.js
index 5f777ac..418cf71 100644
--- a/src/service/courseService.js
+++ b/src/service/courseService.js
@@ -128,17 +128,7 @@ function searchCourseAPI (req, response) {
       })
     },
     function (res, CBW) {
-      if (req.query && req.query.orgdetails && res.result && res.result.content) {
-        var fields = req.query.orgdetails
-        orgHelper.populateOrgDetailsByHasTag(res.result.content, fields, function (err, courseswithorgdetails) {
-          if (!err) {
-            res.result.content = courseswithorgdetails
-          }
-          CBW(null, res)
-        })
-      } else {
-        CBW(null, res)
-      };
+      orgHelper.includeOrgDetails(req, res, CBW)
     },
     function (res) {
       rspObj.result = res.result
@@ -471,17 +461,7 @@ function getCourseAPI (req, response) {
       })
     },
     function (res, CBW) {
-      if (req.query && req.query.orgdetails) {
-        var fields = req.query.orgdetails
-        orgHelper.populateOrgDetailsByHasTag(res.result.content, fields, function (err, courseswithorgdetails) {
-          if (!err) {
-            res.result.content = courseswithorgdetails
-          }
-          CBW(null, res)
-        })
-      } else {
-        CBW(null, res)
-      };
+      orgHelper.includeOrgDetails(req, res, CBW)
     },
     function (res) {
       rspObj.result = transformResBody(res.result, 'content', 'course')
@@ -600,18 +580,7 @@ function getCourseHierarchyAPI (req, response) {
       })
     },
     function (res, CBW) {
-      if (req.query && req.query.orgdetails && res.result && res.result.content) {
-        var fields = req.query.orgdetails
-        // sending res.result.content as array bec populateOrgDetailsByHasTag expects data as array
-        orgHelper.populateOrgDetailsByHasTag([res.result.content], fields, function (err, courseswithorgdetails) {
-          if (!err) {
-            res.result.content = courseswithorgdetails[0]
-          }
-          CBW(null, res)
-        })
-      } else {
-        CBW(null, res)
-      };
+      orgHelper.includeOrgDetails(req, res, CBW)
     },
     function (res) {
       rspObj.result = res.result
-- 
GitLab