Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Facebook sdk #3477

Merged
merged 13 commits into from Jun 5, 2019
Merged

Conversation

MichaelPetrinolis
Copy link
Contributor

Reorganize directory structure of OrchardCore.Facebook module
Integrate sdk.js
Add predefined Social Plugins as widgets with OrchardCode.Facebook.Widgets feature
Rename OrchardCore.Facebook.Core feature to OrchardCore.Facebook (might break existing sites)
Add a guide how to use the Widgets feature
Widgets defined

  • Like
  • Comments
  • Continue with
  • Chat
  • Quote
  • Save
  • Share

The module defines a FacebookPluginPart that is used to define the widgets. Instead we could add settings to the Liquid Part to define a predefined liquid body and an option to render alternates. Then we could use it in this and any other module that defines widgets this way

@Jetski5822
Copy link
Member

Do we know what the licensing is for these libs?

@MichaelPetrinolis
Copy link
Contributor Author

It seems to be like Facebook Login integration. You need a facebook developer account and accept the Facebook's terms and conditions

Copied from https://appsource.microsoft.com/en-us/product/office/WA103354412

The OrchardCore.Facebook.Widgets allows you to install certain social plugins made available by Facebook on your webpages (“Plugins”). Your use of the Plugins is subject to the Facebook Platform Policies and all other applicable terms and conditions as specified by Facebook, which terms may also include a privacy policy. You are responsible for reviewing and complying with all applicable terms specified by Facebook. OrchardCore.Facebook.Widgets is not a party to the license between you and Facebook, and its privacy statement doesn’t apply to treatment of information you may exchange with Facebook when using the Plugins. OrchardCore.Facebook.Widgets isn’t responsible for the Plugins or your use of them, any warranties or claims relating to the Plugins, or customer support for the operation of the Plugins.

@agriffard
Copy link
Member

@MichaelPetrinolis Have you considered to add a Workflow activity that would allow to publish a message on a Facebook page for example?
Are you aware of which SDK permissions it would imply?
Do you know any useful .NET Core supported Facebook library ?

Similarly, it could be a nice feature to add to the Twitter module: A workflow activity to post a tweet.

@MichaelPetrinolis
Copy link
Contributor Author

@agriffard nice idea! After some research, it seems that Facebook provides manage_pages and publish_pages permissions that require app review, but disallows an app to post on behalf of the user. Instead we can use the instant articles which allow the app to publish.(https://developers.facebook.com/docs/instant-articles/api#create)
Unfortunately pages do not work with application tokens, we must implement something in the admin to help and administrator to retrieve a page token for an FB user. In particular, we must get a user token, exchange it for a long lived token and then require a page token that never expires.
I don't know of any .NET Core FB lib, but if we implement such functionality we can do it without adding any dependency.

}
}

if (script is object)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What have you done @Jetski5822

<fieldset class="form-group">
<label asp-for="Liquid">@T["Body"]</label>
<textarea asp-for="Liquid" rows="5" class="form-control content-preview-text"></textarea>
<span class="hint">@T["The body of the content item, in Liquid format."]</span>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The body of what?

templateContext.MemberAccessStrategy.Register<FacebookPluginPartViewModel>();
await templateContext.ContextualizeAsync(shapeDisplayContext.DisplayContext);

using (var writer = new StringWriter())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a helper that already does this for you, and also pools the buffers

@sebastienros
Copy link
Member

We definitely need a demo, but it even has a guide, so it can't be bad

@agriffard
Copy link
Member

@domonkosgabor has tried it and included screenshots here: https://orcharddojo.net/blog/this-week-in-orchard-04-26-2019

add info that tenant will be reloaded if login settings change
fix issues from code review
}
}

if (script == null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it mean we don't invoke the next middleware if it's not null?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was the intention...

var settings = site.As<FacebookSettings>();
if (httpContext.Request.Path.Value.EndsWith("fbsdk.js"))
{
var locale = string.IsNullOrWhiteSpace(site.Culture) ? "en_US" : site.Culture.Replace("-", "_");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This settings doesn't exist anymore. You need to use ILocalizationService and it will give you the DefaultCulture to use.
But I think you should use Globalinfo.CurrentUICulture instead.

d.getElementsByTagName('head')[0].appendChild(js);
}} (document));";
}
if (httpContext.Request.Path.Value.EndsWith("fb.js"))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should use Path.GetFilename of the url

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or the last segment

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or EndsWith("/fb.js")

@@ -1,28 +1,62 @@
using Microsoft.AspNetCore.Authentication;
using Microsoft.AspNetCore.Authentication.Facebook;
using Microsoft.AspNetCore.Mvc;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So many new usings?

use Path.GetFileName for safer filename parsing of the url
split Startup classes based on feature to reduce usings

namespace OrchardCore.Facebook
{
[Feature(FacebookConstants.Features.Login)]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's a new feature, it should have an entry in the manifest

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new feature is the [Feature(FacebookConstants.Features.Widgets)] and has an entry in the manifest :)

image

The core feature contains the settings needed to integrate the FB application and register fb scripts with resource manager
image

The login feature registers the FB Login external provider, using the appid and secret defined in the Application (core module feature)

The Widgets feature registers most of the fb plugins as widgets and a FacebookPlugin part to extend with new/missing fb plugins as widgets

@sebastienros sebastienros merged commit 249043c into OrchardCMS:dev Jun 5, 2019
@sebastienros sebastienros deleted the facebook_sdk branch June 5, 2019 21:21
scleaver pushed a commit to scleaver/OrchardCore that referenced this pull request Aug 22, 2019
MatthijsKrempel pushed a commit to MatthijsKrempel/OrchardCore that referenced this pull request Dec 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants