Bugfix- Fetch parent feature using topLevel flag

[shashankbrgowda]

The current behavior is returning the top-level feature (gene) for all requests, but the desired behavior is to return the immediate parent of the featureId.

[garrettjstevens]

Can we simplify this more by not even calling getFeatureFromId if topLevel is true? i.e. does this change also fix the bug you were seeing?

diff --git a/packages/apollo-collaboration-server/src/features/features.service.ts b/packages/apollo-collaboration-server/src/features/features.service.ts
--- a/packages/apollo-collaboration-server/src/features/features.service.ts
+++ b/packages/apollo-collaboration-server/src/features/features.service.ts
@@ -86,12 +86,14 @@ export class FeaturesService {
       this.logger.error(errMsg)
       throw new NotFoundException(errMsg)
     }
+    if (topLevel) {
+      return topLevelFeature
+    }
 
     // Now we need to find correct top level feature or sub-feature inside the feature
     const foundFeature = this.getFeatureFromId(
       topLevelFeature,
       featureId,
-      topLevel,
     )
     if (!foundFeature) {
       const errMsg = 'ERROR when searching feature by featureId'
@@ -111,8 +113,6 @@ export class FeaturesService {
   getFeatureFromId(
     feature: Feature,
     featureId: string,
-    topLevel?: boolean,
-    parent?: Feature | null,
   ): Feature | null {
     this.logger.verbose(`Entry=${JSON.stringify(feature)}`)
 
@@ -120,9 +120,6 @@ export class FeaturesService {
       this.logger.debug(
         `Top level featureId matches in object ${JSON.stringify(feature)}`,
       )
-      if (topLevel && parent) {
-        return parent
-      }
       return feature
     }
     // Check if there is also childFeatures in parent feature and it's not empty
@@ -134,13 +131,8 @@ export class FeaturesService {
       const subFeature = this.getFeatureFromId(
         childFeature,
         featureId,
-        topLevel,
-        feature,
       )
       if (subFeature) {
-        if (topLevel) {
-          return feature
-        }
         return subFeature
       }
     }

[shashankbrgowda]

Can we simplify this more by not even calling getFeatureFromId if topLevel is true? i.e. does this change also fix the bug you were seeing?

diff --git a/packages/apollo-collaboration-server/src/features/features.service.ts b/packages/apollo-collaboration-server/src/features/features.service.ts
--- a/packages/apollo-collaboration-server/src/features/features.service.ts
+++ b/packages/apollo-collaboration-server/src/features/features.service.ts
@@ -86,12 +86,14 @@ export class FeaturesService {
       this.logger.error(errMsg)
       throw new NotFoundException(errMsg)
     }
+    if (topLevel) {
+      return topLevelFeature
+    }
 
     // Now we need to find correct top level feature or sub-feature inside the feature
     const foundFeature = this.getFeatureFromId(
       topLevelFeature,
       featureId,
-      topLevel,
     )
     if (!foundFeature) {
       const errMsg = 'ERROR when searching feature by featureId'
@@ -111,8 +113,6 @@ export class FeaturesService {
   getFeatureFromId(
     feature: Feature,
     featureId: string,
-    topLevel?: boolean,
-    parent?: Feature | null,
   ): Feature | null {
     this.logger.verbose(`Entry=${JSON.stringify(feature)}`)
 
@@ -120,9 +120,6 @@ export class FeaturesService {
       this.logger.debug(
         `Top level featureId matches in object ${JSON.stringify(feature)}`,
       )
-      if (topLevel && parent) {
-        return parent
-      }
       return feature
     }
     // Check if there is also childFeatures in parent feature and it's not empty
@@ -134,13 +131,8 @@ export class FeaturesService {
       const subFeature = this.getFeatureFromId(
         childFeature,
         featureId,
-        topLevel,
-        feature,
       )
       if (subFeature) {
-        if (topLevel) {
-          return feature
-        }
         return subFeature
       }
     }

This will return topLevelFeature (gene) for all requests. We need to return immediate parent of requested featureId.

Sorry for the confusion with description, I've updated it now.