From 7b9910f251e139012b48eae6f63e86cd7c1cdc62 Mon Sep 17 00:00:00 2001
From: Thomas Citharel <tcit@tcit.fr>
Date: Wed, 24 Mar 2021 10:45:29 +0100
Subject: [PATCH] Resources fixes and improvements
- Fix getting page description
- Fix fetching metadata from Twitter (thx @marienfressinaud)
- Improve error handling
Signed-off-by: Thomas Citharel <tcit@tcit.fr>
---
js/src/views/Resources/ResourceFolder.vue | 35 +++++++++++++------
lib/graphql/resolvers/resource.ex | 9 ++++-
lib/service/rich_media/parser.ex | 18 ++++++++--
.../rich_media/parsers/meta_tags_parser.ex | 10 ++++--
4 files changed, 56 insertions(+), 16 deletions(-)
diff --git a/js/src/views/Resources/ResourceFolder.vue b/js/src/views/Resources/ResourceFolder.vue
index 83873920d..645b03ffb 100644
--- a/js/src/views/Resources/ResourceFolder.vue
+++ b/js/src/views/Resources/ResourceFolder.vue
@@ -190,6 +190,9 @@
<b-modal :active.sync="createLinkResourceModal" has-modal-card>
<div class="modal-card">
<section class="modal-card-body">
+ <b-message type="is-danger" v-if="modalError">
+ {{ modalError }}
+ </b-message>
<form @submit.prevent="createResource">
<b-field :label="$t('URL')">
<b-input
@@ -317,6 +320,8 @@ export default class Resources extends Mixins(ResourceMixin) {
renameModal = false;
+ modalError = "";
+
groupObject: Record<string, unknown> = {
name: "resources",
pull: "clone",
@@ -341,6 +346,7 @@ export default class Resources extends Mixins(ResourceMixin) {
async createResource(): Promise<void> {
if (!this.resource.actor) return;
+ this.modalError = "";
try {
await this.$apollo.mutate({
mutation: CREATE_RESOURCE,
@@ -364,21 +370,28 @@ export default class Resources extends Mixins(ResourceMixin) {
this.newResource.resourceUrl = "";
} catch (err) {
console.error(err);
+ this.modalError = err.graphQLErrors[0].message;
}
}
async previewResource(): Promise<void> {
- if (this.newResource.resourceUrl === "") return;
- const { data } = await this.$apollo.mutate({
- mutation: PREVIEW_RESOURCE_LINK,
- variables: {
- resourceUrl: this.newResource.resourceUrl,
- },
- });
- this.newResource.title = data.previewResourceLink.title;
- this.newResource.summary = data.previewResourceLink.description;
- this.newResource.metadata = data.previewResourceLink;
- this.newResource.type = "link";
+ this.modalError = "";
+ try {
+ if (this.newResource.resourceUrl === "") return;
+ const { data } = await this.$apollo.mutate({
+ mutation: PREVIEW_RESOURCE_LINK,
+ variables: {
+ resourceUrl: this.newResource.resourceUrl,
+ },
+ });
+ this.newResource.title = data.previewResourceLink.title;
+ this.newResource.summary = data.previewResourceLink.description;
+ this.newResource.metadata = data.previewResourceLink;
+ this.newResource.type = "link";
+ } catch (err) {
+ console.error(err);
+ this.modalError = err.graphQLErrors[0].message;
+ }
}
createSentenceForType(type: string): string {
diff --git a/lib/graphql/resolvers/resource.ex b/lib/graphql/resolvers/resource.ex
index 16e5040b7..13b43ba84 100644
--- a/lib/graphql/resolvers/resource.ex
+++ b/lib/graphql/resolvers/resource.ex
@@ -118,6 +118,9 @@ defmodule Mobilizon.GraphQL.Resolvers.Resource do
) do
{:ok, resource}
else
+ {:error, _} ->
+ {:error, dgettext("errors", "Error while creating resource")}
+
{:own_check, _} ->
{:error, dgettext("errors", "Parent resource doesn't belong to this group")}
@@ -201,8 +204,12 @@ defmodule Mobilizon.GraphQL.Resolvers.Resource do
{:ok, data} when is_map(data) ->
{:ok, struct(Metadata, data)}
- {:error, _err} ->
+ {:error, :invalid_parsed_data} ->
+ {:error, dgettext("errors", "Unable to fetch resource details from this URL.")}
+
+ {:error, err} ->
Logger.warn("Error while fetching preview from #{inspect(resource_url)}")
+ Logger.debug(inspect(err))
{:error, :unknown_resource}
end
end
diff --git a/lib/service/rich_media/parser.ex b/lib/service/rich_media/parser.ex
index 738d19620..2ea91f995 100644
--- a/lib/service/rich_media/parser.ex
+++ b/lib/service/rich_media/parser.ex
@@ -57,7 +57,7 @@ defmodule Mobilizon.Service.RichMedia.Parser do
@spec parse_url(String.t(), Enum.t()) :: {:ok, map()} | {:error, any()}
defp parse_url(url, options \\ []) do
- user_agent = Keyword.get(options, :user_agent, Config.instance_user_agent())
+ user_agent = Keyword.get(options, :user_agent, default_user_agent(url))
headers = [{"User-Agent", user_agent}]
Logger.debug("Fetching content at address #{inspect(url)}")
@@ -212,7 +212,8 @@ defmodule Mobilizon.Service.RichMedia.Parser do
end
defp check_parsed_data(data) do
- {:error, "Found metadata was invalid or incomplete: #{inspect(data)}"}
+ Logger.debug("Found metadata was invalid or incomplete: #{inspect(data)}")
+ {:error, :invalid_parsed_data}
end
defp clean_parsed_data(data) do
@@ -293,4 +294,17 @@ defmodule Mobilizon.Service.RichMedia.Parser do
end
defp check_remote_picture_path(data), do: data
+
+ # Twitter requires a well-know crawler user-agent to show server-rendered data
+ defp default_user_agent("https://twitter.com/" <> _) do
+ "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)"
+ end
+
+ defp default_user_agent("https://mobile.twitter.com/" <> _) do
+ "Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)"
+ end
+
+ defp default_user_agent(_url) do
+ Config.instance_user_agent()
+ end
end
diff --git a/lib/service/rich_media/parsers/meta_tags_parser.ex b/lib/service/rich_media/parsers/meta_tags_parser.ex
index bd383db71..316e444c5 100644
--- a/lib/service/rich_media/parsers/meta_tags_parser.ex
+++ b/lib/service/rich_media/parsers/meta_tags_parser.ex
@@ -70,8 +70,14 @@ defmodule Mobilizon.Service.RichMedia.Parsers.MetaTagsParser do
defp maybe_put_description(meta, html) when meta != %{} do
case get_page_description(html) do
- "" -> meta
- description -> Map.put_new(meta, :description, description)
+ "" ->
+ meta
+
+ descriptions when is_list(descriptions) and length(descriptions) > 0 ->
+ Map.put_new(meta, :description, hd(descriptions))
+
+ description ->
+ Map.put_new(meta, :description, description)
end
end