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.

Sign up for GitHub

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

Jump to bottom

Feature/article canonical urls #1378

Closed
wants to merge 30 commits into from
Closed

Feature/article canonical urls #1378

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
a762c17
feat: adding canonical Urls for articles in topic and author-profile
Sep 26, 2018
9572a9f
chore: refactor tests
Sep 26, 2018
3a1c1f4
fix:linting
Sep 26, 2018
bf26c76
chore: fix linting
Sep 26, 2018
aa63001
chore: making fixture data more relevant
Sep 26, 2018
2592fad
chore: ordering vars alphabetically
Sep 26, 2018
f039da9
chore: refactor
Sep 26, 2018
e83098f
chore: refactor
Sep 26, 2018
ba3b6cd
chore: fix linting
Sep 26, 2018
61ce753
chore: remove render specific feature
Sep 26, 2018
17235bc
chore: update prop type
Sep 26, 2018
405f90a
fix: passing makeurl in context
Sep 27, 2018
47a87a3
chore: adding canUrl to RA
Sep 27, 2018
761b495
chore: refactor
Sep 27, 2018
f96ce68
chore: remove makeurl as a prop
Sep 27, 2018
1a6fe88
chore: refactor
Sep 27, 2018
d62c06a
chore: fix broken test
Sep 28, 2018
1667338
fix: adding Context for ArticleList tests
Oct 1, 2018
86294be
lint and changes to ssr
Oct 1, 2018
6516deb
chore: fixing tests
Oct 2, 2018
1dcf9f5
chore: updating Pages with Context
Oct 2, 2018
db12b59
chore: refactor
Oct 2, 2018
76b731f
chore: refactor
Oct 2, 2018
a989c39
chore: update storybook for composed components
Oct 2, 2018
fec9095
chore: updating fixtures
Oct 2, 2018
99b7816
chore: rebasing and linting
Oct 3, 2018
1cde2d3
chore: rebasing and linting
Oct 3, 2018
de3b2f9
chore: refactor import
Oct 4, 2018
a1e9572
fix: make url function part of util package
Oct 8, 2018
31f4ca6
chore: add new file
Oct 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/article-list/__tests__/android/__snapshots__/article-list.android.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ Array [
"publishedTime": "2017-11-17T00:01:00.000Z",
"section": "world",
"shortHeadline": "ShortHeadline 1",
"shortIdentifier": "968n7tdck1",
"shortSummary": Array [
Object {
"attributes": Object {},
Expand All @@ -278,6 +279,7 @@ Array [
"name": "paragraph",
},
],
"slug": "this-is-slug-1",
"summary": Array [
Object {
"attributes": Object {},
Expand Down Expand Up @@ -327,6 +329,7 @@ Array [
"publicationName": "SUNDAYTIMES",
"publishedTime": "2017-11-17T00:01:00.000Z",
"shortHeadline": "ShortHeadline 2",
"shortIdentifier": "968n7tdck2",
"shortSummary": Array [
Object {
"attributes": Object {},
Expand All @@ -342,6 +345,7 @@ Array [
"name": "paragraph",
},
],
"slug": "this-is-slug-2",
"summary": Array [
Object {
"attributes": Object {},
Expand Down
4 changes: 4 additions & 0 deletions packages/article-list/__tests__/ios/__snapshots__/article-list.ios.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,7 @@ Array [
"publishedTime": "2017-11-17T00:01:00.000Z",
"section": "world",
"shortHeadline": "ShortHeadline 1",
"shortIdentifier": "968n7tdck1",
"shortSummary": Array [
Object {
"attributes": Object {},
Expand All @@ -278,6 +279,7 @@ Array [
"name": "paragraph",
},
],
"slug": "this-is-slug-1",
"summary": Array [
Object {
"attributes": Object {},
Expand Down Expand Up @@ -327,6 +329,7 @@ Array [
"publicationName": "SUNDAYTIMES",
"publishedTime": "2017-11-17T00:01:00.000Z",
"shortHeadline": "ShortHeadline 2",
"shortIdentifier": "968n7tdck2",
"shortSummary": Array [
Object {
"attributes": Object {},
Expand All @@ -342,6 +345,7 @@ Array [
"name": "paragraph",
},
],
"slug": "this-is-slug-2",
"summary": Array [
Object {
"attributes": Object {},
Expand Down
18 changes: 11 additions & 7 deletions packages/article-list/__tests__/shared-error.base.web.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import React from "react";
import TestRenderer from "react-test-renderer";
import { makeUrl } from "@times-components/utils";
import Context from "@times-components/context";
import { iterator } from "@times-components/test-utils";
import ArticleListPageError from "../src/article-list-page-error";
import ArticleList from "../src/article-list";
Expand Down Expand Up @@ -32,13 +34,15 @@ export default () => {
name: "article list",
test() {
const testInstance = TestRenderer.create(
<ArticleList
adConfig={adConfig}
articles={articlesFixture.slice(0, 2)}
emptyStateMessage="Empty state"
pageSize={3}
refetch={() => {}}
/>
<Context.Provider value={{ makeUrl }}>
<ArticleList
adConfig={adConfig}
articles={articlesFixture.slice(0, 2)}
emptyStateMessage="Empty state"
pageSize={3}
refetch={() => {}}
/>
</Context.Provider>
);

expect(testInstance).toMatchSnapshot();
Expand Down
62 changes: 36 additions & 26 deletions packages/article-list/__tests__/shared-states.web.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React from "react";
import { Text } from "react-native";
import TestRenderer from "react-test-renderer";
import Context from "@times-components/context";
import { makeUrl } from "@times-components/utils";
import { iterator } from "@times-components/test-utils";
import {
addSerializers,
Expand Down Expand Up @@ -40,12 +42,14 @@ export default () => {
test() {
const apolloError = new ApolloError("Test");
const testInstance = TestRenderer.create(
<ArticleList
adConfig={adConfig}
emptyStateMessage="Empty State"
error={apolloError}
refetch={() => {}}
/>
<Context.Provider value={{ makeUrl }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

Why don't we add this as a default value instead?

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 reason I did not put it in the default is that then it becomes the default way to do it, whereas a noop is just saying this is just a default and client's responsible for providing it

<ArticleList
adConfig={adConfig}
emptyStateMessage="Empty State"
error={apolloError}
refetch={() => {}}
/>
</Context.Provider>
);

expect(testInstance).toMatchSnapshot();
Expand All @@ -56,13 +60,15 @@ export default () => {
test() {
const apolloError = new ApolloError("Test");
const testInstance = TestRenderer.create(
<ArticleList
adConfig={adConfig}
articleListHeader={<Text>Some Header</Text>}
emptyStateMessage="Empty State"
error={apolloError}
refetch={() => {}}
/>
<Context.Provider value={{ makeUrl }}>
<ArticleList
adConfig={adConfig}
articleListHeader={<Text>Some Header</Text>}
emptyStateMessage="Empty State"
error={apolloError}
refetch={() => {}}
/>
</Context.Provider>
);

expect(testInstance).toMatchSnapshot();
Expand All @@ -72,12 +78,14 @@ export default () => {
name: "an empty list",
test() {
const testInstance = TestRenderer.create(
<ArticleList
adConfig={adConfig}
articles={[]}
emptyStateMessage="Empty state"
refetch={() => {}}
/>
<Context.Provider value={{ makeUrl }}>
<ArticleList
adConfig={adConfig}
articles={[]}
emptyStateMessage="Empty state"
refetch={() => {}}
/>
</Context.Provider>
);

expect(testInstance).toMatchSnapshot();
Expand All @@ -87,13 +95,15 @@ export default () => {
name: "loading state",
test() {
const testInstance = TestRenderer.create(
<ArticleList
adConfig={adConfig}
articlesLoading
emptyStateMessage="Empty state"
pageSize={3}
refetch={() => {}}
/>
<Context.Provider value={{ makeUrl }}>
<ArticleList
adConfig={adConfig}
articlesLoading
emptyStateMessage="Empty state"
pageSize={3}
refetch={() => {}}
/>
</Context.Provider>
);

expect(testInstance).toMatchSnapshot();
Expand Down
18 changes: 11 additions & 7 deletions packages/article-list/__tests__/shared-tracking.web.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import React, { Component } from "react";
import PropTypes from "prop-types";
import TestRenderer from "react-test-renderer";
import { makeUrl } from "@times-components/utils";
import Context from "@times-components/context";
import { iterator } from "@times-components/test-utils";
import ArticleList from "../src/article-list";
import articlesFixture from "../fixtures/articles.json";
Expand All @@ -26,13 +28,15 @@ export default () => {

render() {
return (
<ArticleList
adConfig={adConfig}
articles={articlesFixture}
emptyStateMessage="Empty state"
pageSize={3}
refetch={() => {}}
/>
<Context.Provider value={{ makeUrl }}>
<ArticleList
adConfig={adConfig}
articles={articlesFixture}
emptyStateMessage="Empty state"
pageSize={3}
refetch={() => {}}
/>
</Context.Provider>
);
}
}
Expand Down
16 changes: 10 additions & 6 deletions packages/article-list/__tests__/shared.base.web.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import React from "react";
import TestRenderer from "react-test-renderer";
import Context from "@times-components/context";
import { makeUrl } from "@times-components/utils";
import { iterator } from "@times-components/test-utils";
import ArticleList from "../src/article-list";
import articlesFixture from "../fixtures/articles.json";
Expand All @@ -26,12 +28,14 @@ export default (additionalTests = []) => {
name: "article list",
test() {
const testInstance = TestRenderer.create(
<ArticleList
adConfig={adConfig}
articles={articlesFixture}
emptyStateMessage="Empty state"
refetch={() => {}}
/>
<Context.Provider value={{ makeUrl }}>
<ArticleList
adConfig={adConfig}
articles={articlesFixture}
emptyStateMessage="Empty state"
refetch={() => {}}
/>
</Context.Provider>
);

expect(testInstance).toMatchSnapshot();
Expand Down
38 changes: 22 additions & 16 deletions packages/article-list/__tests__/shared.web.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import {
minimalWebTransform,
print
} from "@times-components/jest-serializer";
import Context from "@times-components/context";
import { makeUrl } from "@times-components/utils";
import TestRenderer from "react-test-renderer";
import "./mocks";
import { omitWeb as omitProps } from "./utils";
Expand All @@ -31,14 +33,16 @@ export default () => {
name: "article list with no images",
test() {
const testInstance = TestRenderer.create(
<ArticleList
adConfig={adConfig}
articles={articlesFixture.slice(0, 1)}
emptyStateMessage="Empty state"
pageSize={3}
refetch={() => {}}
showImages={false}
/>
<Context.Provider value={{ makeUrl }}>
<ArticleList
adConfig={adConfig}
articles={articlesFixture.slice(0, 1)}
emptyStateMessage="Empty state"
pageSize={3}
refetch={() => {}}
showImages={false}
/>
</Context.Provider>
);

expect(testInstance).toMatchSnapshot();
Expand All @@ -57,14 +61,16 @@ export default () => {
};

const testInstance = TestRenderer.create(
<ArticleList
adConfig={adConfig}
articles={[missingImage]}
emptyStateMessage="Empty state"
pageSize={3}
refetch={() => {}}
showImages={false}
/>
<Context.Provider value={{ makeUrl }}>
<ArticleList
adConfig={adConfig}
articles={[missingImage]}
emptyStateMessage="Empty state"
pageSize={3}
refetch={() => {}}
showImages={false}
/>
</Context.Provider>
);

expect(testInstance).toMatchSnapshot();
Expand Down
12 changes: 6 additions & 6 deletions packages/article-list/__tests__/web/__snapshots__/article-list.web.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ exports[`1. article list 1`] = `
>
<div>
<Link
url="https://article1.io"
url="https://www.thetimes.co.uk/article/this-is-slug-1-968n7tdck1"
>
<div>
<Card
Expand Down Expand Up @@ -60,7 +60,7 @@ exports[`1. article list 1`] = `
<div />
</div>
<Link
url="https://article2.io"
url="https://www.thetimes.co.uk/article/this-is-slug-2-968n7tdck2"
>
<div>
<Card
Expand Down Expand Up @@ -120,7 +120,7 @@ exports[`1. article list 1`] = `
<div />
</div>
<Link
url="https://article3.io"
url="https://www.thetimes.co.uk/article/this-is-slug-3-968n7tdck3"
>
<div>
<Card
Expand Down Expand Up @@ -180,7 +180,7 @@ exports[`1. article list 1`] = `
<div />
</div>
<Link
url="https://article4.io"
url="https://www.thetimes.co.uk/article/this-is-slug-4-968n7tdck4"
>
<div>
<Card
Expand Down Expand Up @@ -265,7 +265,7 @@ exports[`2. article list with no images 1`] = `
>
<div>
<Link
url="https://article1.io"
url="https://www.thetimes.co.uk/article/this-is-slug-1-968n7tdck1"
>
<div>
<Card
Expand Down Expand Up @@ -348,7 +348,7 @@ exports[`3. article list with missing image 1`] = `
>
<div>
<Link
url="https://article1.io"
url="https://www.thetimes.co.uk/article/this-is-slug-1-968n7tdck1"
>
<div>
<Card
Expand Down