Merge pull request #1228 from smartdevicelink/bugfix/issue-1220-ChoiceSetManager-return-nil-if-noMenuName

Choice set manager should return nil if there's no menu name

5 files changed, 51 insertions, 9 deletions

diff --git a/SmartDeviceLink/SDLError.h b/SmartDeviceLink/SDLError.h
index 59c65c174..cdc2bab41 100644
--- a/SmartDeviceLink/SDLError.h
+++ b/SmartDeviceLink/SDLError.h
@@ -67,6 +67,7 @@ extern SDLErrorDomain *const SDLErrorDomainRPCStore;
 + (NSError *)sdl_choiceSetManager_choicesDeletedBeforePresentation:(NSDictionary *)userInfo;
 + (NSError *)sdl_choiceSetManager_choiceDeletionFailed:(NSDictionary *)userInfo;
 + (NSError *)sdl_choiceSetManager_choiceUploadFailed:(NSDictionary *)userInfo;
++ (NSError *)sdl_choiceSetManager_failedToCreateMenuItems;
 
 #pragma mark Transport
 
diff --git a/SmartDeviceLink/SDLError.m b/SmartDeviceLink/SDLError.m
index 87b6e2237..187940201 100644
--- a/SmartDeviceLink/SDLError.m
+++ b/SmartDeviceLink/SDLError.m
@@ -228,6 +228,15 @@ SDLErrorDomain *const SDLErrorDomainRPCStore = @"com.sdl.rpcStore.error";
     return [NSError errorWithDomain:SDLErrorDomainChoiceSetManager code:SDLChoiceSetManagerErrorUploadFailed userInfo:userInfo];
 }
 
++ (NSError *)sdl_choiceSetManager_failedToCreateMenuItems {
+    NSDictionary<NSString *, NSString *> *userInfo = @{
+                                                       NSLocalizedDescriptionKey: NSLocalizedString(@"Choice Set Manager error", nil),
+                                                       NSLocalizedFailureReasonErrorKey: NSLocalizedString(@"Choice set manager failed to create menu items due to menuName being empty.", nil),
+                                                        NSLocalizedRecoverySuggestionErrorKey: NSLocalizedString(@"If you are setting the menuName, it is possible that the head unit is sending incorrect displayCapabilities.", nil)
+                                                       };
+    return [NSError errorWithDomain:SDLErrorDomainChoiceSetManager code:SDLChoiceSetManagerErrorFailedToCreateMenuItems userInfo:userInfo];
+}
+
 #pragma mark Transport
 
 + (NSError *)sdl_transport_unknownError {
diff --git a/SmartDeviceLink/SDLErrorConstants.h b/SmartDeviceLink/SDLErrorConstants.h
index c86ee2946..8cbed860e 100644
--- a/SmartDeviceLink/SDLErrorConstants.h
+++ b/SmartDeviceLink/SDLErrorConstants.h
@@ -123,6 +123,7 @@ typedef NS_ENUM(NSInteger, SDLChoiceSetManagerError) {
     SDLChoiceSetManagerErrorPendingPresentationDeleted = -1,
     SDLChoiceSetManagerErrorDeletionFailed = -2,
     SDLChoiceSetManagerErrorUploadFailed = -3,
+    SDLChoiceSetManagerErrorFailedToCreateMenuItems = -4
 };
 
 /**
diff --git a/SmartDeviceLink/SDLPreloadChoicesOperation.m b/SmartDeviceLink/SDLPreloadChoicesOperation.m
index 1e9be8ec9..8fe9718cb 100644
--- a/SmartDeviceLink/SDLPreloadChoicesOperation.m
+++ b/SmartDeviceLink/SDLPreloadChoicesOperation.m
@@ -118,9 +118,18 @@ NS_ASSUME_NONNULL_BEGIN
 
     NSMutableArray<SDLCreateInteractionChoiceSet *> *choiceRPCs = [NSMutableArray arrayWithCapacity:self.cellsToUpload.count];
     for (SDLChoiceCell *cell in self.cellsToUpload) {
-        [choiceRPCs addObject:[self sdl_choiceFromCell:cell]];
+        SDLCreateInteractionChoiceSet *csCell =  [self sdl_choiceFromCell:cell];
+        if(csCell != nil) {
+            [choiceRPCs addObject:csCell];
+        }
     }
-
+    if (choiceRPCs.count == 0) {
+        SDLLogE(@"All choice cells to send are nil, so the choice set will not be shown");
+        self.internalError = [NSError sdl_choiceSetManager_failedToCreateMenuItems];
+        [self finishOperation];
+        return;
+    }
+    
     __weak typeof(self) weakSelf = self;
     __block NSMutableDictionary<SDLRPCRequest *, NSError *> *errors = [NSMutableDictionary dictionary];
     [self.connectionManager sendRequests:[choiceRPCs copy] progressHandler:^(__kindof SDLRPCRequest * _Nonnull request, __kindof SDLRPCResponse * _Nullable response, NSError * _Nullable error, float percentComplete) {
@@ -141,15 +150,20 @@ NS_ASSUME_NONNULL_BEGIN
 
 #pragma mark - Assembling Choice Data
 
-- (SDLCreateInteractionChoiceSet *)sdl_choiceFromCell:(SDLChoiceCell *)cell {
+- (nullable SDLCreateInteractionChoiceSet *)sdl_choiceFromCell:(SDLChoiceCell *)cell {
     NSArray<NSString *> *vrCommands = nil;
     if (cell.voiceCommands == nil) {
         vrCommands = self.isVROptional ? nil : @[[NSString stringWithFormat:@"%hu", cell.choiceId]];
     } else {
         vrCommands = cell.voiceCommands;
     }
-
+    
     NSString *menuName = [self.displayCapabilities hasTextFieldOfName:SDLTextFieldNameMenuName] ? cell.text : nil;
+    if(!menuName) {
+        SDLLogE(@"Could not convert SDLChoiceCell to SDLCreateInteractionChoiceSet. It will not be shown. Cell: %@", cell);
+        return nil;
+    }
+    
     NSString *secondaryText = [self.displayCapabilities hasTextFieldOfName:SDLTextFieldNameSecondaryText] ? cell.secondaryText : nil;
     NSString *tertiaryText = [self.displayCapabilities hasTextFieldOfName:SDLTextFieldNameTertiaryText] ? cell.tertiaryText : nil;
 
diff --git a/SmartDeviceLinkTests/DevAPISpecs/SDLPreloadChoicesOperationSpec.m b/SmartDeviceLinkTests/DevAPISpecs/SDLPreloadChoicesOperationSpec.m
index 6c9d7793d..5bcf2d5de 100644
--- a/SmartDeviceLinkTests/DevAPISpecs/SDLPreloadChoicesOperationSpec.m
+++ b/SmartDeviceLinkTests/DevAPISpecs/SDLPreloadChoicesOperationSpec.m
@@ -46,6 +46,9 @@ describe(@"a preload choices operation", ^{
         beforeEach(^{
             displayCapabilities = [[SDLDisplayCapabilities alloc] init];
             displayCapabilities.graphicSupported = @YES;
+            SDLTextField *primaryTextField = [[SDLTextField alloc] init];
+            primaryTextField.name = SDLTextFieldNameMenuName;
+            displayCapabilities.textFields = @[primaryTextField];
 
             OCMStub([testFileManager uploadArtworks:[OCMArg any] completionHandler:[OCMArg invokeBlock]]);
         });
@@ -69,6 +72,21 @@ describe(@"a preload choices operation", ^{
                 cellsWithArtwork = [NSSet setWithArray:@[cell1WithArt, cell2WithArtAndSecondary]];
                 cellsWithStaticIcon = [NSSet setWithArray:@[cellWithStaticIcon]];
             });
+            
+            context(@"if the menuName is not set", ^{
+                it(@"should not send any requests", ^{
+                    SDLTextField *primaryTextField = [[SDLTextField alloc] init];
+                    primaryTextField.name = SDLTextFieldNameMenuName;
+                    displayCapabilities.textFields = @[];
+
+                    testOp = [[SDLPreloadChoicesOperation alloc] initWithConnectionManager:testConnectionManager fileManager:testFileManager displayCapabilities:displayCapabilities isVROptional:NO cellsToPreload:cellsWithArtwork];
+                    [testOp start];
+                
+                    NSArray<SDLCreateInteractionChoiceSet *> *receivedRequests = (NSArray<SDLCreateInteractionChoiceSet *> *)testConnectionManager.receivedRequests;
+                    
+                    expect(receivedRequests).to(haveCount(0));
+                });
+            });
 
             context(@"disallowed display capabilities", ^{
                 it(@"should skip to preloading cells", ^{
@@ -163,8 +181,7 @@ describe(@"a preload choices operation", ^{
             beforeEach(^{
                 SDLChoiceCell *cellBasic = [[SDLChoiceCell alloc] initWithText:@"Cell1" artwork:nil voiceCommands:nil];
                 SDLChoiceCell *cellWithVR = [[SDLChoiceCell alloc] initWithText:@"Cell2" secondaryText:nil tertiaryText:nil voiceCommands:@[@"Cell2"] artwork:nil secondaryArtwork:nil];
-                 SDLChoiceCell *cellWithAllText = [[SDLChoiceCell alloc] initWithText:@"Cell2" secondaryText:@"Cell2" tertiaryText:@"Cell2" voiceCommands:nil artwork:nil secondaryArtwork:nil];
-
+                SDLChoiceCell *cellWithAllText = [[SDLChoiceCell alloc] initWithText:@"Cell2" secondaryText:@"Cell2" tertiaryText:@"Cell2" voiceCommands:nil artwork:nil secondaryArtwork:nil];
                 cellsWithoutArtwork = [NSSet setWithArray:@[cellBasic, cellWithVR, cellWithAllText]];
             });
 
@@ -176,11 +193,10 @@ describe(@"a preload choices operation", ^{
                 it(@"should be correct with no text and VR required", ^{
                     testOp = [[SDLPreloadChoicesOperation alloc] initWithConnectionManager:testConnectionManager fileManager:testFileManager displayCapabilities:displayCapabilities isVROptional:NO cellsToPreload:cellsWithoutArtwork];
                     [testOp start];
-
                     NSArray<SDLCreateInteractionChoiceSet *> *receivedRequests = (NSArray<SDLCreateInteractionChoiceSet *> *)testConnectionManager.receivedRequests;
 
                     expect(receivedRequests).to(haveCount(3));
-                    expect(receivedRequests.lastObject.choiceSet.firstObject.menuName).to(beNil());
+                    expect(receivedRequests.lastObject.choiceSet.firstObject.menuName).toNot(beNil());
                     expect(receivedRequests.lastObject.choiceSet.firstObject.secondaryText).to(beNil());
                     expect(receivedRequests.lastObject.choiceSet.firstObject.tertiaryText).to(beNil());
                     expect(receivedRequests.lastObject.choiceSet.firstObject.vrCommands).toNot(beNil());
@@ -244,13 +260,14 @@ describe(@"a preload choices operation", ^{
                 });
 
                 it(@"should be correct with VR optional", ^{
+
                     testOp = [[SDLPreloadChoicesOperation alloc] initWithConnectionManager:testConnectionManager fileManager:testFileManager displayCapabilities:displayCapabilities isVROptional:YES cellsToPreload:cellsWithoutArtwork];
                     [testOp start];
 
                     NSArray<SDLCreateInteractionChoiceSet *> *receivedRequests = (NSArray<SDLCreateInteractionChoiceSet *> *)testConnectionManager.receivedRequests;
 
                     expect(receivedRequests).to(haveCount(3));
-                    expect(receivedRequests.lastObject.choiceSet.firstObject.menuName).to(beNil());
+                    expect(receivedRequests.lastObject.choiceSet.firstObject.menuName).toNot(beNil());
                     expect(receivedRequests.lastObject.choiceSet.firstObject.secondaryText).to(beNil());
                     expect(receivedRequests.lastObject.choiceSet.firstObject.tertiaryText).to(beNil());
                     expect(receivedRequests.lastObject.choiceSet.firstObject.vrCommands).to(beNil());