Reintroduce "Handle /release requests" #239

MartinMalinda · 2017-06-27T20:53:26Z

make /lts and /release work again
make tests pass
make sure it works well with FastBoot
add rel=canonical meta to pages with versions that match release or lts
add test coverage for canonicalURL

sivakumar-kailasam · 2017-06-27T21:25:01Z

@mschinis will this be ok to have with regards to algolia indexing?

mschinis · 2017-06-28T17:52:52Z

@sivakumar-kailasam yep, I believe we can ignore specific paths as adding both /release and /ember/2.13.3/classes/Ember.Evented/methods/off would cause duplicate algolia results

MartinMalinda · 2017-06-28T18:17:35Z

I'm for leaving out /lts and /release from the search results. If someone is doing a search it's likely for some method/event/property which might be version specific.

mschinis · 2017-06-28T18:20:37Z

@MartinMalinda are you going to redirect /lts and /release to the appropriate url?

MartinMalinda · 2017-06-28T18:51:51Z

@mschinis Probably not so far. So far I am not convinced we should do that. This PR actually goes a little bit into opposite direction.

If you visit /ember/release it makes sure that after clicking further links, you stay on /ember/release. So that if you then share that link on other places, it keeps linking to the latest version. It also helps SEO if we have one main url that is most dominantly shared. /release than should appear on top in search results (hopefully).

I'm open to other suggestions here though. I can be wrong.

sivakumar-kailasam · 2017-06-28T19:16:41Z

@mschinis if we're in a particular version of docs and if we start performing an aloglia search, are results shown to the current version's docs first?

+1 on not redirecting to specific versions since too many redirects night penalize us on SEO. On a totally unrelated note I tried reaching existing api path structures on api-beta and whatever I tried redirected as expected.

mschinis · 2017-06-28T21:57:30Z

@MartinMalinda

Agreed on keeping the url the same, but we need to make sure we get this right, as it can actually screw up with SEO. If /ember/release and /ember/2.13.0 offer the same content, then Google will think that the content is duplicate. In the release variant, we would have to set-up rel="canonical" tags in the header to point to the original document (2.13.0)

@sivakumar-kailasam yes, the search should only search through the version in the URL, so we might actually need to get algolia to also scrape the current URL. In my previous message above, I thought we were only going to redirect, hence why I mentioned we could ignore specific URLs.

MartinMalinda · 2017-06-29T07:03:02Z

@mschinis there was an idea to have nofollow for this but rel="canonical" seems like a better choice. I'll see if it can easily be added to this PR otherwise new issue will have to be created.

MartinMalinda · 2017-06-30T22:37:41Z

Ok, it seems like it is possible to populate headData from the metaStore, which contains project versions. So conditional rendering of <link> should be possible. I'll finish it tomorrow hopefully.

sivakumar-kailasam · 2017-07-01T06:23:33Z

@MartinMalinda can you rearrange your commits a bit to get a cleaner history?

mschinis · 2017-07-01T11:47:08Z

@MartinMalinda I'm going to add ember-cli-meta-tags to add a lot more <link> tags to improve our SEO. You can use that plugin to also introduce the canonical link tags.

MartinMalinda · 2017-07-01T11:48:56Z

@mschinis I have the canonical URL mostly done, I'm just improving test coverage. Can refactor it later though.

MartinMalinda · 2017-07-01T12:19:51Z

canonicalURL works now, but there is an issue when currentURL is null on the client side. Which causes the canonicalURL to be wiped after booting the app on the client side.

https://github.com/ember-learn/ember-api-docs/pull/239/files#diff-8be18fa18c3ed63d667abc0d4ae71015R33

Issue: ember-polyfills/ember-router-service-polyfill#2

MartinMalinda · 2017-07-01T21:54:38Z

I think this is almost ready to merge. The only thing missing is test coverage for canonicalURL which is hard to do without mirage or any other backend mock. But I'll give it some more thought.

MartinMalinda · 2017-07-04T19:45:00Z

Seems like the only proper way to test the canonicalURL is to use https://github.com/kaliber5/ember-fastboot-app-tests.

toddjordan · 2017-07-11T04:11:21Z

@MartinMalinda do you mind resolving conflicts? I'd like to get this reviewed/tested.

…ersions

sivakumar-kailasam · 2017-07-12T10:20:19Z

tests/acceptance/release-lts-project-versions-test.js

+    return checkLinks('attributes', '.attributes a', versionString);
+  };
+
+  let checkIndexListLinks = versionString => {


not a big deal but this could also be written as,

let checkIndexListLinks = (versionString) => checkLinks('index-list', '.api-index-filter a', versionString);

👍 will change that unless the line is too long

Nah this is fine :)

Just changed it :D it made me notice this flaw here f37e79d#diff-b409b67c33eb7131aee5de1bebe70e5bR14 at least. Passed selector variable was actually not used in the helper function.

sivakumar-kailasam · 2017-07-12T10:27:29Z

app/models/project-version.js

+    return this.get('project.latestProjectVersion.id') === this.get('version');
+  }),
+
+  isLTS: computed(function() {


Missed linking to version in the computed definition

The version on project-version model does not change, but maybe I should add it anyway for clarity.

Oh yeah that makes sense. Missed the context there :)

toddjordan · 2018-01-20T17:53:38Z

we ended up taking a slightly different approach, but currently have release supported. What we still need to do is add a feature that puts canonical to release in each non-release page. May need some thought on how to do this for pre-2.16 versions that don't match names/modules.

MartinMalinda changed the title ~~[WIP] Reintroduce "Handle /release requests (#209)"~~ [WIP] Reintroduce "Handle /release requests" Jun 27, 2017

MartinMalinda force-pushed the release-param-reintroduction branch 2 times, most recently from 4b0ba74 to faadfa0 Compare July 1, 2017 08:44

MartinMalinda mentioned this pull request Jul 1, 2017

Links between projects appear to be broken #243

Closed

MartinMalinda changed the title ~~[WIP] Reintroduce "Handle /release requests"~~ Reintroduce "Handle /release requests" Jul 1, 2017

locks temporarily deployed to ember-api-docs-staging-pr-239 July 11, 2017 04:02 Inactive

MartinMalinda mentioned this pull request Jul 11, 2017

Replace link-to component with href-to helper #252

Closed

MartinMalinda added 7 commits July 11, 2017 19:00

Reintroduce "Handle /release requests (ember-learn#209)"

807c2f1

move the /release acceptance test to async await

b121c9c

use ember-router-service-polyfill and fix eslint

c1e2389

fix output of sortedProjectVersions property to contain all project v…

c6596ba

…ersions

move semver sorting to metaStore service

b88f88e

remove last version util and use latestProjectVersion prop instead

bf87ef5

use Ember.set to update metaStore and do it only once

f2385c2

MartinMalinda added 5 commits July 11, 2017 19:17

create canonical URL if needed

580ec87

move testest semVer sorting in different unit tests

fe2f20a

set canonicalURL later when currentURL is available

1869152

refactor canonicalURL into computed property

0b064f2

add post-rebase fixes

3d4343d

MartinMalinda force-pushed the release-param-reintroduction branch from 12b8d4f to 3d4343d Compare July 11, 2017 17:51

fix remaining tests

c08974d

locks requested a deployment to ember-api-docs-staging-pr-239 July 12, 2017 08:28 Pending

locks temporarily deployed to ember-api-docs-staging-pr-239 July 12, 2017 08:28 Inactive

locks requested a deployment to ember-api-docs-staging-pr-239 July 12, 2017 08:29 Pending

locks temporarily deployed to ember-api-docs-staging-pr-239 July 12, 2017 08:29 Inactive

locks temporarily deployed to ember-api-docs-staging-pr-239 July 12, 2017 08:35 Inactive

sivakumar-kailasam reviewed Jul 12, 2017

View reviewed changes

improve test quality

f37e79d

locks temporarily deployed to ember-api-docs-staging-pr-239 July 12, 2017 11:14 Inactive

This was referenced Jul 15, 2017

Add LTS label to relevant version in the version picker #288

Closed

Consider adding canonicalURL to all older versions #289

Open

comment canonical URL so far

a26be7f

locks deployed to ember-api-docs-staging-pr-239 July 15, 2017 12:39 View deployment

sivakumar-kailasam requested a review from toddjordan July 16, 2017 16:06

sivakumar-kailasam approved these changes Jul 16, 2017

View reviewed changes

toddjordan closed this Jan 20, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reintroduce "Handle /release requests" #239

Reintroduce "Handle /release requests" #239

MartinMalinda commented Jun 27, 2017 •

edited

Loading

sivakumar-kailasam commented Jun 27, 2017

mschinis commented Jun 28, 2017

MartinMalinda commented Jun 28, 2017

mschinis commented Jun 28, 2017

MartinMalinda commented Jun 28, 2017

sivakumar-kailasam commented Jun 28, 2017

mschinis commented Jun 28, 2017

MartinMalinda commented Jun 29, 2017

MartinMalinda commented Jun 30, 2017

sivakumar-kailasam commented Jul 1, 2017

mschinis commented Jul 1, 2017

MartinMalinda commented Jul 1, 2017

MartinMalinda commented Jul 1, 2017 •

edited

Loading

MartinMalinda commented Jul 1, 2017

MartinMalinda commented Jul 4, 2017

toddjordan commented Jul 11, 2017

sivakumar-kailasam Jul 12, 2017

MartinMalinda Jul 12, 2017

sivakumar-kailasam Jul 12, 2017

MartinMalinda Jul 12, 2017

sivakumar-kailasam Jul 12, 2017 •

edited

Loading

MartinMalinda Jul 12, 2017

sivakumar-kailasam Jul 12, 2017

toddjordan commented Jan 20, 2018

Reintroduce "Handle /release requests" #239

Reintroduce "Handle /release requests" #239

Conversation

MartinMalinda commented Jun 27, 2017 • edited Loading

sivakumar-kailasam commented Jun 27, 2017

mschinis commented Jun 28, 2017

MartinMalinda commented Jun 28, 2017

mschinis commented Jun 28, 2017

MartinMalinda commented Jun 28, 2017

sivakumar-kailasam commented Jun 28, 2017

mschinis commented Jun 28, 2017

MartinMalinda commented Jun 29, 2017

MartinMalinda commented Jun 30, 2017

sivakumar-kailasam commented Jul 1, 2017

mschinis commented Jul 1, 2017

MartinMalinda commented Jul 1, 2017

MartinMalinda commented Jul 1, 2017 • edited Loading

MartinMalinda commented Jul 1, 2017

MartinMalinda commented Jul 4, 2017

toddjordan commented Jul 11, 2017

sivakumar-kailasam Jul 12, 2017

Choose a reason for hiding this comment

MartinMalinda Jul 12, 2017

Choose a reason for hiding this comment

sivakumar-kailasam Jul 12, 2017

Choose a reason for hiding this comment

MartinMalinda Jul 12, 2017

Choose a reason for hiding this comment

sivakumar-kailasam Jul 12, 2017 • edited Loading

Choose a reason for hiding this comment

MartinMalinda Jul 12, 2017

Choose a reason for hiding this comment

sivakumar-kailasam Jul 12, 2017

Choose a reason for hiding this comment

toddjordan commented Jan 20, 2018

MartinMalinda commented Jun 27, 2017 •

edited

Loading

MartinMalinda commented Jul 1, 2017 •

edited

Loading

sivakumar-kailasam Jul 12, 2017 •

edited

Loading