[deliver] fix iPad 12.9" 3rd gen screenshot identifier for App Store Connect API #17364
Conversation
|
I can't see what failed the build on CI above ☝️ but what's failing locally is something related to the Google Play Store, which is also present in master 😬 not sure how to proceed |
|
Does this request has been merged? I have faced with same issue. |
Not yet, @niyaoyao ! If you're looking for a temporary workaround, check my comment here #16975 (comment) - hope it helps you! I believe this PR will be merged soon, though. 🙏 |
Co-authored-by: Satoshi Namai <[email protected]>
|
@joshdholtz could you take a look at this one? 🤗 It's been causing new issues to be open e.g. #18045 |
|
On it! |
|
Ping @joshdholtz 🤗 |
|
Another friendly ping 🙈 @joshdholtz |
|
@rogerluan could you fix the conflicts so we hopefully soon can get this out? |
# Conflicts: # deliver/lib/deliver/app_screenshot.rb c
| "iPad Pro (12.9-inch) (3rd generation)", # default simulator name has this | ||
| "iPad Pro (12.9-inch) (4th generation)", # default simulator name has this | ||
| "ipadPro129", # downloaded screenshots name has this, | ||
| "3GEN" # downloaded screenshots name from App Store Connect API has this |
There was a problem hiding this comment.
Nevermind, I just saw that this change was added by @joshdholtz 😁 29598bc
I think it's safe to include the entire identifier IPAD_PRO_3GEN_129 🙏 but I don't mind keeping just 3GEN either
|
Done @dinsen 💪 |
|
Friendly ping @joshdholtz 🤗 |
|
Ping @joshdholtz ? 🙈 |
|
Wait what... this never got merged?! |
|
The root issue actually got fixed by another PR a few months ago 😬 so this PR now just adds a unit test to cover that case, and documents the change @joshdholtz 🙇 |
|
Hey @rogerluan 👋 Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉 Please let us know if this change requires an immediate release by adding a comment here 👍 |
fastlane-bot
added
status: included-in-next-release
and removed
status: needs-attention
labels
There was a problem hiding this comment.
Congratulations! 🎉 This was released as part of fastlane 2.197.0 🚀
🔑
Checklist
bundle exec rspecfrom the root directory to see all new and existing tests passbundle exec rubocop -ato ensure the code style is validMotivation and Context
Resolves #16975
Resolves #18045
More context in this comment of mine: #16975 (comment)
Description
It appears that since the App Store Connect API, the name of screenshots downloaded from App Store Connect of the iPad 12.9" 3rd gen was changed to
%d_APP_IPAD_PRO_3GEN_129_%d.Since there's no logic to handle that identifier out of the box (i.e. if you download the screenshots using deliver and try to re-upload them, it won't work unless you rename the iPad 12.9" 3rd gen screenshots according to the docs), this PR fixes that, maintaining the old name identifiers, but also adding support to
IPAD_PRO_3GEN_129.This PR also improves some copy and documentation in general, that helps people better understand this caveat of the iPad screenshots.
Testing Steps
This issue could be reproduced by downloading your screenshots from App Store Connect using deliver, and try re-uploading them. fastlane doesn't know that it has to upload those screenshots and assign them to the iPad 12.9" 3rd gen, so it will attempt to upload them as regular iPad 12.9" 2nd gen screenshots, which isn't accepted by Apple during review time (it could potentially get metadata rejected - I've witnessed this).
To test this branch, modify your Gemfile as: