Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep File Manager from Error State on disallowed #1455

Merged
merged 11 commits into from Oct 24, 2019

Conversation

justingluck93
Copy link
Contributor

@justingluck93 justingluck93 commented Oct 21, 2019

Fixes #1454

This PR is ready for review.

Risk

This PR makes no API changes.

Testing Plan

Test against example app using Xcode 11. Make sure to remove ListFilesRPC from cores policy table in order to recreate a disallowed

Summary

This PR adds a check to see if the error was disallowed. If it is disallowed we no longer want to Error but rather continue with the setup process.

CLA

@justingluck93 justingluck93 added the bug A defect in the library label Oct 21, 2019
@justingluck93 justingluck93 self-assigned this Oct 21, 2019
@justingluck93 justingluck93 added this to In progress in v6.4 via automation Oct 21, 2019
@justingluck93 justingluck93 changed the title making it so we dont error on a DISALLOWED Keep File Manager from Error State on disallowed Oct 22, 2019
SmartDeviceLink/SDLFileManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLFileManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLFileManager.m Outdated Show resolved Hide resolved
v6.4 automation moved this from In progress to Review in progress Oct 22, 2019
SmartDeviceLink/SDLListFilesOperation.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLFileManager.m Outdated Show resolved Hide resolved
joeljfischer
joeljfischer previously approved these changes Oct 23, 2019
v6.4 automation moved this from Review in progress to Reviewer approved Oct 23, 2019
@joeljfischer joeljfischer self-requested a review October 23, 2019 17:27
@joeljfischer joeljfischer dismissed their stale review October 23, 2019 17:27

Needs additional testing

v6.4 automation moved this from Reviewer approved to Review in progress Oct 23, 2019
Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add unit tests to the ListFilesOperationSpec and the FileManager spec to account for this case.

SmartDeviceLink/SDLFileManager.m Outdated Show resolved Hide resolved
justingluck93 and others added 2 commits October 24, 2019 08:45
Co-Authored-By: Joel Fischer <joeljfischer@gmail.com>
@@ -157,6 +157,22 @@ @implementation FileManagerSpecHelper
});
});

describe(@"after receiving a ListFiles error with a resulCode", ^{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
describe(@"after receiving a ListFiles error with a resulCode", ^{
describe(@"after receiving a ListFiles error with a resultCode DISALLOWED", ^{

@@ -59,7 +59,14 @@ - (void)sdl_listFiles {
NSUInteger bytesAvailable = listFilesResponse.spaceAvailable != nil ? listFilesResponse.spaceAvailable.unsignedIntegerValue : 2000000000;

if (weakSelf.completionHandler != nil) {
weakSelf.completionHandler(success, bytesAvailable, fileNames, error);
if([response.resultCode isEqualToEnum:SDLResultDisallowed]) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this, just always set the resultCode into the error if an error exists.

expect(@(successResult)).to(equal(@NO));
});

it(@"should have called completion handler with error including a resultCode ", ^{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the above change request, I think that this it is the only one needed?

Copy link
Contributor

@joeljfischer joeljfischer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just two very minor comment changes

SmartDeviceLink/SDLFileManager.m Outdated Show resolved Hide resolved
SmartDeviceLink/SDLFileManager.m Outdated Show resolved Hide resolved
Co-Authored-By: Joel Fischer <joeljfischer@gmail.com>
v6.4 automation moved this from Review in progress to Reviewer approved Oct 24, 2019
@joeljfischer joeljfischer merged commit 80d298f into develop Oct 24, 2019
v6.4 automation moved this from Reviewer approved to Done Oct 24, 2019
@joeljfischer joeljfischer deleted the bugfix/issue-1454-ListFiles-Disallowed branch October 24, 2019 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A defect in the library
Projects
No open projects
v6.4
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants