Skip to content

[iOS & tvOS] Upgrade SDK to 10.10 #1463

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

Merged
merged 25 commits into from
Apr 7, 2025
Merged

[iOS & tvOS] Upgrade SDK to 10.10 #1463

merged 25 commits into from
Apr 7, 2025

Conversation

JPKribs
Copy link
Member

@JPKribs JPKribs commented Mar 28, 2025

Summary

Indirectly resolves: #1289 & #1293

This PR serves as a starting point to migrate us from the current 10.8 SDK to 10.10. No actual 10.10 features are added except for the previously built and commented out DeviceDetailsView & AccessTags items that were already built in preparation for 10.10.

Other than those 2 views, everything else is just a 1:1 update to get everything working and buildable on 10.10. I am using my own branch for the 10.10 SDK but I have a pull request to hopefully get that in here.

This PR should serve as a good starting point but we likely need to knock out some TODOs first.

TODOs

  • I have some outstanding items in this PR that are commented with TODO: 10.10 -. None of these are preventing building nor do they causes any issues in initial testing but they should be resolved first. These primarily have to do with the new init for UserPolicy so it should be very very quick.

@JPKribs JPKribs added enhancement New feature or request developer Improves or alters the developer experience labels Mar 28, 2025
@JPKribs JPKribs changed the title [WIP] [iOS & tvOS] Upgrade SDK to 10.10 [iOS & tvOS] Upgrade SDK to 10.10 Mar 28, 2025
@JPKribs JPKribs marked this pull request as ready for review March 28, 2025 06:03
Copy link
Member Author

@JPKribs JPKribs left a comment

Choose a reason for hiding this comment

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

Just putting in some comments to get ahead of potential questions/reasons.

@LePips
Copy link
Member

LePips commented Mar 30, 2025

I can't remember if there were any actual issues that caused me to close #1072, but if there aren't any regressions this can go in 1.3.

@JPKribs
Copy link
Member Author

JPKribs commented Mar 30, 2025

if there aren't any regressions this can go in 1.3.

Not that I'm aware of lol. That being said, my reasoning for 1.4 is that I don't know how much longer we want to keep the on TestFlight for 1.3 going. I'd hate to have some critical Live TV bug or something that is floating around in 10.10 for us that I missed because I don't have Live TV configured or something like that.


Last revision in. I was just thinking about the getUserView how there isn't a UserId input for it anymore. Went back and found that UserId lives in the getUserViewParameters now. So I just updated the 2 spots this changed.

This should be ready again for a review!

@LePips
Copy link
Member

LePips commented Mar 30, 2025

I'm not that concerned that 1.3 has been in TestFlight for as long as it has, at least not as much as I was at first. Since we're dropping iOS 15, putting in as many bug fixes would be desirable. We can still have bug fixes for iOS 15 in 1.3.x updates after dropping support by working in a separate branch, but I want to keep any fixes that could potentially happen to a minimum.

@JPKribs
Copy link
Member Author

JPKribs commented Mar 30, 2025

Okay one more change. I didn't realize you started this in #1072 so I took a look a this. I preferred how you were using SupportedCaseIterable over there and that was much much cleaner than what I was doing. So, I've moved those same changes into this one.


Unless you see anything, I think that moves me to only 2 outstanding questions:

  1. What was deviceProfile.responseProfile and why doesn't it exist anymore? Does it impact us? In testing transcoding I don't see any issues even with custom profiles but I want to be sure there isn't anything missing.
  2. UserPolicy now requires authenticationProviderID: "" & passwordResetProviderID: "" to init. Do we want to use that as the fallback or should these just error? All of these are in the Admin Dashboard. Which, if it somehow did go to a point where UserPolicy was empty that would cause some issues on the server side since I believe all the updates to UserPolicy take the whole UserPolicy as an input. As a result, if we ever init a new UserPolicy, it would wipe the user's UserPolicy instead of incrementally changing it. For that reason, I might lean towards a UserPolicy! or Guard instead of these. That being said, authenticationProviderID: "" & passwordResetProviderID: "" as fallbacks don't change any functionality.

Unofficial 3), do we want to change this up now while we're thinking about it: #1463 (comment)

JPKribs added 3 commits March 30, 2025 15:07
Resolved: // TODO: 10.10 - Filter to only valid SortBy's for each BaseItemKind.
Last outstanding item: // TODO: 10.10 - What should authenticationProviderID & passwordResetProviderID be?
@JPKribs
Copy link
Member Author

JPKribs commented Apr 7, 2025

The route I took was using:

        guard let policy = viewModel.user.policy else {
            preconditionFailure("User policy cannot be empty.")
        }
        
        self.tempPolicy = policy

The UserPolicy should ALWAYS exist if you get to this point. If it does not, something is wrong. If, for whatever reason, there is no UserPolicy or the UserPolicy gets lost somehow, initializing a new one will cause issues. If I have a UserPolicy, and Swiftfin initializes a new one with only the new changes, this UserPolicy would override the one on the server and would remove all settings except the new one.

For this reason, I chose to go the preconditionFailure route. I believe I am using that correctly but please let me know if that is incorrect.

If this is correct, 10.10 should be ready to go!

Copy link
Member

@LePips LePips left a comment

Choose a reason for hiding this comment

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

Great work!

@LePips
Copy link
Member

LePips commented Apr 7, 2025

We will have to revisit the addition of preconditionFailures to prevent error states instead of crashing the app.

@LePips LePips merged commit 0025422 into jellyfin:main Apr 7, 2025
3 of 4 checks passed
@JPKribs JPKribs deleted the SDK1010 branch April 7, 2025 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer Improves or alters the developer experience enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[iOS] [10.10] Sesssions - Transcoding Parsing Issues
2 participants