search

IDEMSInternational / open-app-builder

PLH App Frontend
GNU General Public License v3.0
5 stars 24 forks source link

chore: update ios build files and template #2275

Closed jfmcquade closed 3 months ago

jfmcquade commented 3 months ago

PR Checklist

Description

Follow-up to #2234

After using xcode to build and publish an iOS app to App Store Connect, some of the iOS build files were updated. This PR includes those updates in the templated file.

The specific changes enacted by xcode were:

Git Issues

Closes #

jfmcquade commented 3 months ago

@chrismclarke I've requested your review although I appreciate you're not really able to test, I just thought it could be good to sense-check

jfmcquade commented 3 months ago

I can also see DEVELOPMENT_TEAM = CHLBG6683N; in a couple places which I'm assuming is the appstore id for the idems developer account (?) and maybe also should be set by a variable?

That's right, and I think that setting it from a variable probably makes sense. Although it's not clear to me where the value of that variable should be specified – it seems to be semi-secret and so maybe shouldn't be in the deployment config, although I can't find a source that states that (chatGPT suggests Apple don't say so explicitly). It could just be a repo- (or organisation-) level secret/variable, but without a local counterpart it would be an additional step for devs to populate that value.

I'm guessing there will also be some updates required when adding xcode API permissions, but happy to just merge for now in any case and iterate as we go

Yes indeed. And things could get even more complicated if we try to remove functionality from certain apps related to API permissions. We've already got a case where the plh_facilitator_mx deployment, which we're publishing to the app store, doesn't use notifications but still requests permissions (see #2265). I know we've previously discussed removing code that is unused by a given app in order to optimise that bundle size. We'd also eventually ideally have the relevant permissions removed from the relevant places in the iOS configuration where possible, so that we weren't requesting permissions with Apple for features that aren't used for a given deployment.

chrismclarke commented 3 months ago

That's right, and I think that setting it from a variable probably makes sense. Although it's not clear to me where the value of that variable should be specified – it seems to be semi-secret and so maybe shouldn't be in the deployment config, although I can't find a source that states that (chatGPT suggests Apple don't say so explicitly). It could just be a repo- (or organisation-) level secret/variable, but without a local counterpart it would be an additional step for devs to populate that value.

Yeah I think that's potentially something partially overlooked with the current variable handling system. I guess the safest way is to just use an ios.config.json file that is encrypted and stores any sensitive data. Alternatively it might be worth supporting a deployment-level .env file which can be populated from input prompts when trying to populate template files (or populate from github env when running on CI).

Although for the specific case of development_team the following StackOverflow post suggests it's not actually all that sensitive: https://stackoverflow.com/questions/44417171/should-i-keep-the-apple-developer-team-id-secret

Yes indeed. And things could get even more complicated if we try to remove functionality from certain apps related to API permissions. We've already got a case where the plh_facilitator_mx deployment, which we're publishing to the app store, doesn't use notifications but still requests permissions (see #2265). I know we've previously discussed removing code that is unused by a given app in order to optimise that bundle size. We'd also eventually ideally have the relevant permissions removed from the relevant places in the iOS configuration where possible, so that we weren't requesting permissions with Apple for features that aren't used for a given deployment.

Yeah that does start to get awkward. I know expo uses a novel system where it automatically generates android/ios core files each time, fully populating all code from inspection of config file (a bit like how capacitor populates app names, ids and a few other things when adding platform) - however they have a whole community repo of specific plugin configurations to handle things like the relevant firebase bom as required, so maintaining our own system like that would be pretty tricky...

At least if focusing on opt-out only then we can use comments in template files to remove as necessary (so core build starts with all possible permissions and code stripped away depending on specific parameters)