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.

Sign up for GitHub

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

Jump to bottom

Remove deprecated attributes from apps requests #21187

Merged
merged 4 commits into from
Apr 16, 2023
+5 −5
Merged

Remove deprecated attributes from apps requests #21187

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
269c82f
Remove prices from ESSENTIAL_INCLUDES closes #21125
giginet Apr 6, 2023
4c47150
Fix spec
giginet Apr 6, 2023
2193c8d
Fix violations
giginet Apr 6, 2023
66d18a6
Fix stubs
giginet Apr 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions spaceship/lib/spaceship/connect_api/models/app.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ module EducationDiscountType
"contentRightsDeclaration" => "content_rights_declaration",

"appStoreVersions" => "app_store_versions",
# This attribute is already deprecated. It will be removed in a future release.
"prices" => "prices"
Copy link
Collaborator Author

@giginet giginet Apr 6, 2023 •
edited

Choose a reason for hiding this comment

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

I have no idea whether we can drop this attribute.

We have to migrate for a latest API

Copy link

Choose a reason for hiding this comment

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

As I see, old applications continue working with old attributes, so it is better to leave it, for backward compatibility.

Copy link
Contributor

Choose a reason for hiding this comment

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

I disagree, this attribute should be removed as with this PR's changes it will always end up being nil. Part of the code seeing the nil might think this app has no price even though it might have prices set up in App Store Connect, ending up with confusion like in #21213.
Not having the attribute would catch possible misuses.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I might have misunderstood part of the code, as it seems app data can come from different locations, some with or without prices. The current state is pretty confusing 😅

})

ESSENTIAL_INCLUDES = [
"appStoreVersions",
"prices"
"appStoreVersions"
].join(",")

def self.type
Expand Down
2 changes: 1 addition & 1 deletion spaceship/lib/spaceship/tunes/tunes_client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ def applications
# if it needs to
#
# https://github.com/fastlane/fastlane/pull/20480
r = request(:get, "https://appstoreconnect.apple.com/iris/v1/apps?include=appStoreVersions,prices")
r = request(:get, "https://appstoreconnect.apple.com/iris/v1/apps?include=appStoreVersions")
response = Spaceship::ConnectAPI::Response.new(
body: r.body,
status: r.status,
Expand Down
4 changes: 2 additions & 2 deletions spaceship/spec/connect_api/testflight/testflight_stubbing.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@ def stub_apps
stub_request(:get, "https://appstoreconnect.apple.com/iris/v1/apps").
to_return(status: 200, body: read_fixture_file('apps.json'), headers: { 'Content-Type' => 'application/json' })

stub_request(:get, "https://appstoreconnect.apple.com/iris/v1/apps?include=appStoreVersions,prices").
stub_request(:get, "https://appstoreconnect.apple.com/iris/v1/apps?include=appStoreVersions").
to_return(status: 200, body: read_fixture_file('apps.json'), headers: { 'Content-Type' => 'application/json' })

stub_request(:get, "https://appstoreconnect.apple.com/iris/v1/apps?filter%5BbundleId%5D=com.joshholtz.FastlaneTest&include=appStoreVersions,prices").
stub_request(:get, "https://appstoreconnect.apple.com/iris/v1/apps?filter%5BbundleId%5D=com.joshholtz.FastlaneTest&include=appStoreVersions").
to_return(status: 200, body: read_fixture_file('apps.json'), headers: { 'Content-Type' => 'application/json' })

stub_request(:get, "https://appstoreconnect.apple.com/iris/v1/apps/123456789").
Expand Down