diff --git a/app/controllers/articles_controller.rb b/app/controllers/articles_controller.rb index b222eed9b..ee3117e6f 100644 --- a/app/controllers/articles_controller.rb +++ b/app/controllers/articles_controller.rb @@ -185,6 +185,7 @@ class ArticlesController < ApplicationController def stats authorize current_user, :pro_user? authorize @article + @organization_id = @article.organization_id end private diff --git a/app/javascript/analytics/dashboard.js b/app/javascript/analytics/dashboard.js index b3afb1cf9..c8facfeef 100644 --- a/app/javascript/analytics/dashboard.js +++ b/app/javascript/analytics/dashboard.js @@ -1,4 +1,5 @@ import Chart from 'chart.js'; +import handleFetchAPIErrors from '../src/utils/errors'; function resetActive(activeButton) { const buttons = document.getElementsByClassName('timerange-button'); @@ -177,11 +178,14 @@ function callAnalyticsApi(date, timeRangeLabel, { organizationId, articleId }) { } fetch(url) - .then(data => data.json()) + .then(handleFetchAPIErrors) + .then(response => response.json()) .then(data => { drawCharts(data, timeRangeLabel); writeCards(data, timeRangeLabel); - }); + }) + // eslint-disable-next-line no-console + .catch(error => console.error(error)); // we should come up with better error handling } function drawWeekCharts({ organizationId, articleId }) { diff --git a/app/javascript/packs/analyticsArticle.js b/app/javascript/packs/analyticsArticle.js index 36b71717a..be5390f81 100644 --- a/app/javascript/packs/analyticsArticle.js +++ b/app/javascript/packs/analyticsArticle.js @@ -2,7 +2,8 @@ import initCharts from '../analytics/dashboard'; function initDashboardArticle() { const article = document.getElementById('article'); - initCharts({ articleId: article.dataset.articleId }); + const { articleId, organizationId } = article.dataset; + initCharts({ articleId, organizationId }); } window.InstantClick.on('change', () => { diff --git a/app/javascript/src/utils/errors.js b/app/javascript/src/utils/errors.js new file mode 100644 index 000000000..290ad9458 --- /dev/null +++ b/app/javascript/src/utils/errors.js @@ -0,0 +1,21 @@ +// eslint-disable-next-line consistent-return +export default function handleFetchAPIErrors(response) { + // pass along a correct response + if (response.ok) { + return response; + } + + // API errors contain the error message in {"error": "error message"} + // but they could be unhandled 500 errors + try { + response.json().then(data => { + throw new Error(data.error); + }); + } catch (e) { + if (e instanceof SyntaxError) { + throw new Error(response.statusText); + } else { + throw e; + } + } +} diff --git a/app/services/analytics_service.rb b/app/services/analytics_service.rb index 6dd40f22c..1a64cec3e 100644 --- a/app/services/analytics_service.rb +++ b/app/services/analytics_service.rb @@ -62,7 +62,7 @@ class AnalyticsService @article_data = @article_data.where(id: @article_id) # check article_id is published and belongs to the user/org - raise ArgumentError unless @article_data.exists? + raise ArgumentError, "You can't view this article's stats" unless @article_data.exists? article_ids = [@article_id] else diff --git a/app/views/articles/stats.html.erb b/app/views/articles/stats.html.erb index b3f4216f7..60bfdf56b 100644 --- a/app/views/articles/stats.html.erb +++ b/app/views/articles/stats.html.erb @@ -5,7 +5,8 @@ href="<%= @article.current_state_path %>" class="rounded-btn rounded-btn--transparent" id="article" - data-article-id="<%= @article.id %>"> + data-article-id="<%= @article.id %>" + data-organization-id="<%= @organization_id %>"> 👈 Back to the article