fix: allow firebase auth to be enabled independent of crashlytics

[jfmcquade]

PR Checklist

• PR title descriptive (can be used in release notes)

Description

Currently the firebase service initialises if and only if a firebase config is provided at the deployment config level (config.firebase.config) and at least one feature is explicitly enabled in the firebase deployment config property (e.g. config.firebase.crashlytics.enabled). This PR removes the second condition, so that the firebase service initialises if and only if a firebase config is provided.

This means that, for example, the auth feature can be enabled with firebase as a provider, without explicitly setting any further properties of the firebase deployment config beyond providing a firebaseConfig file.

Git Issues

Closes #

Screenshots/Videos

If useful, provide screenshot or capture to highlight main changes

[chrismclarke]

Following #2634 the auth config was updated to assume enabled if a provider was included. I think likely we can do the same of initialising firebase if a config is provided and avoid confusion of default enabled states which in some cases are true and others are false

I might even be tempted to suggest offering a disabled override if there is good reason for an author to provide config but not enable the functionality (more just for firebase config which encompassed multiple services, and likely just crashlytics as the auth is configured within the auth provider)

Do you think that sounds sensible? Ideally I want a way to avoid overuse of enabled: true and enabled: false settings

[jfmcquade]

Thanks @chrismclarke, that definitely sounds sensible (I was a bit rushed in making this PR initially so hadn't thought it through). I've updated the PR and description.

I might even be tempted to suggest offering a disabled override if there is good reason for an author to provide config but not enable the functionality (more just for firebase config which encompassed multiple services, and likely just crashlytics as the auth is configured within the auth provider)

I haven't acted on this as I wasn't quite sure where you're suggesting the disabled property to be: at the level of config.firebase.disabled or config.firebase.crashlytics.disabled? I'm assuming the former, but as this fix is needed urgently I've kept the changes minimal for now.

[esmeetewinkel]

Functional test passed

[esmeetewinkel]

@chrismclarke Could you confirm whether your requested changes have been addressed? From my perspective this is ready to merge.

[chrismclarke]

Sorry for the delay, this one has been on my backlog for a while and aware it has some conflicts with #2742
I'll take a look now and try get it merged in

[chrismclarke]

Apologies for the delay reviewing.

I still felt it was a little weird to have the top-level firebase config required but then child app config optional and crashlytics config again enforced so I've added a tidying commit with ab11339

Now the deployment-level firebase config is entirely optional, however if an author includes then the child app config is required. Crashlytics is then also an optional, opt-in feature. Further down the line we might want to provide more configuration options both within the firebase config (e.g. advanced crashlytics, auth or analytics config), but for now I think this looks fine for mostly just managing the crashlytics service

Feel free to go ahead and merge if happy @jfmcquade - or tag me for re-review if you want me to change anything

[jfmcquade]

Thanks @chrismclarke, the changes look good so I'm happy to merge