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.

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

[EOSF-839] Make use of fingerprinted osf-assets for OSF Preprint logoSharing #503

Merged

Conversation

adlius
Copy link
Collaborator

@adlius adlius commented Dec 15, 2017

Purpose

Currently, the og:image meta tags for OSF Preprint reference to an incorrect asset. This change should fix that to refer to fingerprinted assets in our osf-assets repo.

Summary of Changes

In theme.js, OSF Preprint's logo.path is no longer referencing to assets in ember-osf-preprints. Instead, it is changed to use the same buildProviderAssetPath method that builds fingerprinted assets urls for all provider assets.
Therefore, environment.js is changed to remove the providers list that includes information that is no longer necessary for building OSF Preprint assets.

Side Effects / Testing Notes

None

Ticket

https://openscience.atlassian.net/browse/EOSF-839

Reviewer Checklist

  • meets requirements
  • easy to understand
  • DRY
  • testable and includes test(s)
  • changes described in CHANGELOG.md

@jamescdavis jamescdavis changed the base branch from develop to release/post-review-actions December 15, 2017 19:35
Copy link
Member

@jamescdavis jamescdavis left a comment

Choose a reason for hiding this comment

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

See inline comment. Also, need to remove pathJoin import.

type: 'image/png',
width: 1200,
height: 630
};
return logo;
Copy link
Member

@jamescdavis jamescdavis Dec 18, 2017

Choose a reason for hiding this comment

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

Could also now just:

return {
    path: buildProviderAssetPath(config, this.get('id'), 'sharing.png', this.get('isDomain')),
    type: 'image/png',
    width: 1200,
    height: 630
};

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 25.894% when pulling 8036ab6 on adlius:og-image into 1a8215a on CenterForOpenScience:release/post-review-actions.

Copy link
Member

@jamescdavis jamescdavis left a comment

Choose a reason for hiding this comment

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

consistency is good!

@jamescdavis jamescdavis merged commit c83a3f1 into CenterForOpenScience:release/post-review-actions Jan 12, 2018
@laurenbarker laurenbarker added this to the 0.117.0 milestone Jan 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants