[spaceship] Allow for removing app from sale by updating territories

[lucgrabowski]

Checklist

• I've run bundle exec rspec from the root directory to see all new and existing tests pass
• I've followed the fastlane code style and run bundle exec rubocop -a to ensure the code style is valid
• I've read the Contribution Guidelines
• I've updated the documentation if necessary.

Motivation and Context

There are 3 discussions with questions about removing apps from sale: #19040, #17032, #19375
This change allows for removing an app from sale by updating a list of territories.

app = ... # fetch app
# 
app.update(attributes:{ "availableInNewTerritories": false }, territory_ids:[], allow_removing_from_sale: true) 

Description

It was not possible to remove app from all territories as app update method expected either nil or an array of territory_ids with at least one territory id.

Just for explain use of availableInNewTerritories:
availableInNewTerritories attribute is responsible for New Countries or Regions checkbox

Testing Steps

Tested by removing an app from sale using code above in irb

[lacostej]

Code looks good. Maybe add documentation to the method stating that empty array removes the app from sale.

Is the Tunes.update that calls patch_app used anywhere (even in popular external actions)? Is there a risk of callers accidentally passing empty arrays earlier that would trigger accidental removing from sale?

[lucgrabowski]

Thanks for review @lacostej . I'll add documentation.

Is the Tunes.update that calls patch_app used anywhere (even in popular external actions)? Is there a risk of callers accidentally passing empty arrays earlier that would trigger accidental removing from sale?

You're right. patch_app isn't called in any other place, but update is called in several places and in one of them in UploadPriceTier it's called with territory_ids and it has to be fixed.

[lacostej]

@lucgrabowski that makes me worried that someone has code somewhere that would cause this unintended behavior. Do we have a better way of rolling this without risking this? Maybe it's acceptable as we can always roll it out... WDYT?

[lucgrabowski]

@lacostej I'm wondering right now if it wouldn't be better to add a param allow_removing_from_sale: false to

update(client: nil, attributes: nil, app_price_tier_id: nil, territory_ids: nil, allow_removing_from_sale: false)

and:

patch_app(app_id: nil, attributes: {}, app_price_tier_id: nil, territory_ids: nil, allow_removing_from_sale: false)

That way if someone has code directly using those methods, we wouldn't break it.

[fastlane-bot]

Hey @lucgrabowski 👋

Thank you for your contribution to fastlane and congrats on getting this pull request merged 🎉
The code change now lives in the master branch, however it wasn't released to RubyGems yet.
We usually ship about once a week, and your PR will be included in the next one.

Please let us know if this change requires an immediate release by adding a comment here 👍
We'll notify you once we shipped a new release with your changes 🚀

[fastlane-bot]

Congratulations! 🎉 This was released as part of fastlane 2.207.0 🚀