882363 - Only enable Facebook functionality if the app id is part of the partner customization.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Contacts, defect)

Description

[[:fabrice] Fabrice Desré]

Currently the facebook app id is hardcoded in apps/communications/contacts/oauth2/js/parameters.js, but this app id can't be shared with all partners.

We need to:
- move the app id to a customizable file, with no default value for now.
- disable all facebook visible features when no app is has been set up in the customization.

Comment 1

[Jason Smith [:jsmith]]

We can fix this for Buri devices, but aren't we past the time to fix this issue for Ikura devices?

Comment 2

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: Pointer to PR 10355

Following the necessary steps for partners to ask for their own facebook app id, we have been requested to move it to a file that is part of the customisation right now.

This first approach moves the appid to the config.json file already present in contacts.

We already have a mechanism to load this file and check if fb is enabled or not, now it has another step that is check if we have a appid set or not.

In order to work with the rest of elements we needed to ask the fb_init + config  js files to the rest of documents that are using the oauth 2.0 authentication against fb.

As well, used this patch to remove from the FTU the fb option if not present the appid.

We will need to check if everything works, (did a quick test on the contacts and ftu), but we will need to check synchronisation, post to wall, etc.

JMC we strongly need your advise here.

Thanks!
F.

Comment 3

[Francisco Jordano [:arcturus] [:francisco]]

Hi folks, above is a POC to check the feasibility and how much work we need to port that.

I've sync with Jose Manuel this morning and we are preparing another patch to try to address this.

Same philosophy, moving the appid to the json file.

Thanks

Comment 4

[Jose Manuel Cantera]

Attachment: Pointer to GH PR

Comment 5

[Francisco Jordano [:arcturus] [:francisco]]

We have been discussing with Yuren, and took a simplier approach.

We don't modify any code, we just add the two places where we setup the services:
config.json where we say if we enable facebook or not and
parameters.js where we specify the services keys

Now this is mapped to partners configuration files in the following way:

communications.json -> apps/communications/contacts/config.json
communications_services.json -> apps/communications/contacts/oauth2/js/parameters.js

Now changes are quite smaller and aligned with partner customisation rules:
https://wiki.mozilla.org/B2G/MarketCustomizations

Comments?

Thanks!

Comment 6

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: Pointer to PR 10355 v2

Comment 7

[Jose Manuel Cantera]

Comment on attachment 761964 [details]
Pointer to PR 10355 v2

thanks Francisco!

Comment 8

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 761964 [details]
Pointer to PR 10355 v2

r=yurenju if you remove parameters.js & config.json from git repository which will be generated in build time.

Comment 9

[Joe Cheng [:jcheng] (please needinfo)]

tef+. this is a blocker for shira release

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I am landing this bug on https://github.com/mozilla-b2g/gaia/pull/10388 now with review nits.

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

master: https://github.com/mozilla-b2g/gaia/commit/5e1c9249134b9296db50b70b85a170e29e0f72cc
v1-train: https://github.com/mozilla-b2g/gaia/commit/ea195edff2f75cb9548404f732566f574ab56abe
v1.0.1: https://github.com/mozilla-b2g/gaia/commit/d2151b9b162aa93554921d249636b36208758ea5

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

v1.0.1 revert: https://github.com/mozilla-b2g/gaia/commit/2a73e5ae94c52f56df86d9f3aceeb7fc5406a808

I didn't uplift this correctly.

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

v1.0.1: https://github.com/mozilla-b2g/gaia/commit/b0e2d4a5b6e16a023ae331320578f5363b1425da

Comment 14

[Francisco Jordano [:arcturus] [:francisco]]

Hi folks,

just one comment about this.

The configuration is separeted in two files, one that says if FB is enabled and the other one that contains the keys and endpoints.

With the current configuration in master we are enabling FB, but the appid is empty cause Mozilla needs to negotiate one, which will be lead to errors.

The Telefonica variant including the app id works perfectly.

Thanks

Comment 15

[Francisco Jordano [:arcturus] [:francisco]]

Asking for uplift to v1-train since this will be affected there.

Comment 16

[Jason Smith [:jsmith]]

Does the changes in this patch require followup bugs to make changes to the partner customization repositories?

Comment 17

[John Ford [:jhford] CET/CEST Berlin Time]

1.1hd: ea195edff2f75cb9548404f732566f574ab56abe

Comment 18

[Jason Smith [:jsmith]]

Please back this out. This just broke every possible branch Gaia is supported on for testing Facebook import functionality on non-customized builds.

Comment 19

[Jason Smith [:jsmith]]

(In reply to Jason Smith [:jsmith] from comment #18)
> Please back this out. This just broke every possible branch Gaia is
> supported on for testing Facebook import functionality on non-customized
> builds.

Actually, let's just quickly fix bug 883344.

Comment 20

[Leo]

Who provides the facebook app id? Phone manufacturer or service provider?

Comment 21

[Jeni]

Can you please provide steps to verify this fix as we will blackbox test from the UI?

Comment 22

[Francisco Jordano [:arcturus] [:francisco]]

@Jeni,

right now the idea is to provide a default fb app id, right now there is a fake facebook id which will show a 'appid invalid', is the responsability of each partner to provide their own keys to guarantee the service.

Thanks,
F.