Skip to content

Commit

Permalink
BAM-2585 Whitelist import, fix type errors, and replace user.assert w…
Browse files Browse the repository at this point in the history 
…ith userAssert (#4)

* BAM-2585 whitelist navigation import and fix type errors

* replace user.assert with userAssert
  • Loading branch information
Phil Winchester committed Feb 14, 2019
1 parent dd9b1ac commit f51d0c8
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 39 deletions.
2 changes: 2 additions & 0 deletions build-system/dep-check-config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,8 @@ exports.rules = [
'src/service/navigation.js',
'extensions/amp-skimlinks/0.1/link-rewriter/link-rewriter-manager.js->' +
'src/service/navigation.js',
'extensions/amp-smartlinks/0.1/link-rewriter/link-rewriter-manager.js->' +
'src/service/navigation.js',
'extensions/amp-list/0.1/amp-list.js->' +
'src/service/xhr-impl.js',
'extensions/amp-form/0.1/amp-form.js->' +
Expand Down
26 changes: 14 additions & 12 deletions extensions/amp-smartlinks/0.1/amp-smartlinks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
*/

import {Services} from '../../../src/services';
import {dict} from '../../../src/utils/object';
import {getData} from './../../../src/event-helper';

import {ENDPOINTS, SMARTLINKS_REWRITER_ID} from './constants';
import {LinkRewriterManager} from './link-rewriter/link-rewriter-manager';
Expand All @@ -29,22 +31,22 @@ export class AmpSmartlinks extends AMP.BaseElement {
constructor(element) {
super(element);

/** @private {!../../../src/service/xhr-impl.Xhr} */
/** @private {?../../../src/service/xhr-impl.Xhr} */
this.xhr_ = null;

/** @private {!../../../src/service/ampdoc-impl.AmpDoc} */
/** @private {?../../../src/service/ampdoc-impl.AmpDoc} */
this.ampDoc_ = null;

/** @private {!./link-rewriter/link-rewriter-manager.LinkRewriterManager} */
/** @private {?./link-rewriter/link-rewriter-manager.LinkRewriterManager} */
this.linkRewriterService_ = null;

/** @private {!./link-rewriter/link-rewriter.LinkRewriter} */
/** @private {?./link-rewriter/link-rewriter.LinkRewriter} */
this.smartLinkRewriter_ = null;

/** @private {?Object} */
this.linkmateOptions_ = null;

/** @private {function({?JsonObject}} */
/** @private {?function({JsonObject})} */
this.fetchLinkmateConfig_ = null;

/** @private {?../../../src/service/viewer-impl.Viewer} */
Expand Down Expand Up @@ -90,7 +92,7 @@ export class AmpSmartlinks extends AMP.BaseElement {
this.linkmate_ = new Linkmate(
this.ampDoc_,
this.xhr_,
this.linkmateOptions_,
this.linkmateOptions_
);
});

Expand All @@ -106,7 +108,7 @@ export class AmpSmartlinks extends AMP.BaseElement {

/**
* API call to retrieve the Narrativ config for this extension.
* @return {?JsonObject}
* @return {?Promise}
* @private
*/
getLinkmateOptions_() {
Expand All @@ -120,7 +122,7 @@ export class AmpSmartlinks extends AMP.BaseElement {
})
.then(res => res.json())
.then(res => {
return res.data[0]['amp_config'];
return getData(res)[0]['amp_config'];
});
}

Expand All @@ -134,7 +136,7 @@ export class AmpSmartlinks extends AMP.BaseElement {
this.xhr_.fetchJson(ENDPOINTS.PAGE_IMPRESSION_ENDPOINT, {
method: 'POST',
ampCors: false,
headers: {'Content-Type': 'application/json'},
headers: dict({'Content-Type': 'application/json'}),
body: payload,
});
}
Expand All @@ -158,11 +160,11 @@ export class AmpSmartlinks extends AMP.BaseElement {

/**
* Build the payload for our page load event.
* @return {!Object}
* @return {!JsonObject}
* @private
*/
buildPageImpressionPayload_() {
return {
return /** @type {!JsonObject} */ ({
'events': [{'is_amp': true}],
'organization_id': this.linkmateOptions_.publisherID,
'organization_type': 'publisher',
Expand All @@ -172,7 +174,7 @@ export class AmpSmartlinks extends AMP.BaseElement {
'previous_url': this.referrer_,
'user_agent': this.ampDoc_.win.navigator.userAgent,
},
};
});
}

/**
Expand Down
4 changes: 2 additions & 2 deletions extensions/amp-smartlinks/0.1/link-rewriter/link-rewriter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import {EVENTS, ORIGINAL_URL_ATTRIBUTE} from './constants';
import {LinkReplacementCache} from './link-replacement-cache';
import {Observable} from '../../../../src/observable';
import {TwoStepsResponse} from './two-steps-response';
import {user} from '../../../../src/log';
import {userAssert} from '../../../../src/log';


/** @typedef {!Array<{anchor: !HTMLElement, replacementUrl: ?string}>}} */
Expand Down Expand Up @@ -192,7 +192,7 @@ export class LinkRewriter {
unknownAnchors.map(anchor => ({anchor, replacementUrl: null}))
);
const twoStepsResponse = this.resolveUnknownLinks_(unknownAnchors);
user().assert(twoStepsResponse instanceof TwoStepsResponse,
userAssert(twoStepsResponse instanceof TwoStepsResponse,
'Invalid response from provided "resolveUnknownLinks" function.' +
'"resolveUnknownLinks" should return an instance of TwoStepsResponse');

Expand Down
12 changes: 6 additions & 6 deletions extensions/amp-smartlinks/0.1/linkmate-options.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {user} from '../../../src/log';
import {userAssert} from '../../../src/log';

/**
* Get the config values from the tag on the amp page
Expand All @@ -34,11 +34,11 @@ export function getConfigOptions(element) {
/**
* The slug used to distinguish Narrativ accounts.
* @param {!Element} element
* @return {string}
* @return {?string}
*/
function getNrtvAccountName_(element) {
const nrtvSlug = element.getAttribute('nrtv-account-name');
user().assert(nrtvSlug,
userAssert(nrtvSlug,
'amp-smartlinks: nrtv-account-name is a required field');

return nrtvSlug ? nrtvSlug.toLowerCase() : null;
Expand All @@ -47,11 +47,11 @@ function getNrtvAccountName_(element) {
/**
* Flag to specify if we are to run our Linkmate service.
* @param {!Element} element
* @return {?string}
* @return {boolean}
*/
function getLinkmateFlag_(element) {
const linkmateEnabled = element.getAttribute('linkmate').toLowerCase();
user().assert(linkmateEnabled,
userAssert(linkmateEnabled,
'amp-smartlinks: linkmate is a required field');

return linkmateEnabled === 'true';
Expand All @@ -60,7 +60,7 @@ function getLinkmateFlag_(element) {
/**
* Flag to mark links as exlusive or not.
* @param {!Element} element
* @return {?string}
* @return {boolean}
*/
function getExclusiveLinksFlag_(element) {
const exclusiveLinks = element.getAttribute('exclusive-links').toLowerCase();
Expand Down
39 changes: 20 additions & 19 deletions extensions/amp-smartlinks/0.1/linkmate.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,28 +14,31 @@
* limitations under the License.
*/

import {deepEquals} from '../../../src/json';
import {dict} from '../../../src/utils/object';

import {ENDPOINTS} from './constants';
import {TwoStepsResponse} from './link-rewriter/two-steps-response';
import {deepEquals} from '../../../src/json';
import {getData} from '../../../src/event-helper';


export class Linkmate {
/**
* @param {!../../../src/service/ampdoc-impl.AmpDoc} ampDoc
* @param {!../../../src/service/xhr-impl.Xhr} xhr
* @param {?../../../src/service/ampdoc-impl.AmpDoc} ampDoc
* @param {?../../../src/service/xhr-impl.Xhr} xhr
* @param {?Object} linkmateOptions
*/
constructor(ampDoc, xhr, linkmateOptions) {
/** @private {!../../../src/service/ampdoc-impl.AmpDoc} */
/** @private {?../../../src/service/ampdoc-impl.AmpDoc} */
this.ampDoc_ = ampDoc;

/** @private {!../../../src/service/xhr-impl.Xhr} */
/** @private {?../../../src/service/xhr-impl.Xhr} */
this.xhr_ = xhr;

/** @private {?boolean} */
this.requestExclusiveLinks_ = linkmateOptions.exclusiveLinks;

/** @private {?int} */
/** @private {?number} */
this.publisherID_ = linkmateOptions.publisherID;

/** @private {?string} */
Expand All @@ -44,7 +47,7 @@ export class Linkmate {
/** @private {!Document|!ShadowRoot} */
this.rootNode_ = this.ampDoc_.getRootNode();

/** @private {!Array<!HTMLElement>} */
/** @private {?Array<!HTMLElement>} */
this.anchorList_ = null;

/** @private {?Array<JsonObject>}*/
Expand Down Expand Up @@ -75,15 +78,15 @@ export class Linkmate {

const asyncMappedLinks = this.postToLinkmate_(anchorList)
.then(res => {
this.linkmateResponse_ = res.data[0]['smart_links'];
this.linkmateResponse_ = getData(res)[0]['smart_links'];
this.anchorList_ = anchorList;
return this.mapLinks_();
});

return new TwoStepsResponse(syncMappedLinks, asyncMappedLinks);
} else { // If we didn't need to make an API call return the synchronous response
this.anchorList_ = anchorList;
return new TwoStepsResponse(syncMappedLinks);
return new TwoStepsResponse(syncMappedLinks, null);
}
}

Expand All @@ -97,18 +100,18 @@ export class Linkmate {
const linksPayload = this.buildLinksPayload_(anchorList);
const editPayload = this.getEditInfo_();

const payload = {
const payload = dict({
'article': editPayload,
'links': linksPayload,
};
});

const fetchUrl = ENDPOINTS.LINKMATE_ENDPOINT.replace(
'.pub_id.', this.publisherID_
'.pub_id.', this.publisherID_.toString()
);
const postOptions = {
method: 'POST',
ampCors: false,
headers: {'Content-Type': 'application/json'},
headers: dict({'Content-Type': 'application/json'}),
body: payload,
};

Expand Down Expand Up @@ -158,21 +161,19 @@ export class Linkmate {
/**
* The API response returns unique links. Map those unique links to as many
* urls in the anchorList as possible. Set the replacement url as a shop-link.
* @return {!Array<JsonObject>}
* @return {!./link-rewriter/link-rewriter.AnchorReplacementList}
* @public
*/
mapLinks_() {
const mappedLinks = this.linkmateResponse_.map(smartLink => {
return this.linkmateResponse_.map(smartLink => {
return Array.prototype.slice.call(this.anchorList_)
.map(anchor => {
return {
anchor,
replacementUrl: anchor[this.linkAttribute_] === smartLink.url ?
replacementUrl: anchor[this.linkAttribute_] === smartLink['url'] ?
`https://shop-links.co/${smartLink['auction_id']}/?amp=true` : null,
};
});
});

return mappedLinks.flat();
})[0];
}
}

0 comments on commit f51d0c8

Please sign in to comment.