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

[EOSF-885] Review action rename #329

Merged
merged 8 commits into from
Dec 20, 2017
Merged

[EOSF-885] Review action rename #329

merged 8 commits into from
Dec 20, 2017

Conversation

adlius
Copy link
Collaborator

@adlius adlius commented Dec 11, 2017 •
edited by jamescdavis

Purpose

OSF-8876 modifies the action API endpoint. This PR renames the Ember model from action to review-action and add tests for the changed model. There is another PR for ember-osf-preprints repo for some minor changes.

Summary of Changes

  1. A bunch of renaming for files and imports.
  2. In the newly renamed adapters/review-action.js, the default behavior of the adapter is overridden with a pathForType function, since the API endpoint for review action inquiry is changed from /v2/actions/ to /v2/actions/reviews.
  3. A bunch of tests for the newly renamed review-action model.

Side Effects / Testing Notes

None

Ticket

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

Reviewer Checklist

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

@adlius adlius changed the title [OSF-885] Review action rename [EOSF-885] Review action rename Dec 11, 2017
@adlius adlius mentioned this pull request Dec 11, 2017
Merged
5 tasks
jamescdavis
jamescdavis suggested changes Dec 11, 2017
View reviewed changes
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.

Please describe these changes in CHANGELOG.md

jamescdavis
jamescdavis approved these changes Dec 13, 2017
View reviewed changes
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.

💯 Looks great! We'll wait to merge until we're sure moderation changes are ready.

CHANGELOG.md
- Rename `action` model to `review-action`
- Also rename related adapter, serializer and tests
- Added model tests for `review-action`
- Customize model adapter
Copy link
Member

Choose a reason for hiding this comment

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

Perfect. Thank you!

laurenbarker
laurenbarker suggested changes Dec 15, 2017
View reviewed changes
Copy link
Contributor

@laurenbarker laurenbarker left a comment

Choose a reason for hiding this comment

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

@adlius @jamescdavis can you update the relationship on the user too?

https://github.com/adlius/ember-osf/blob/reviewActionRename/addon/models/user.js#L40

@jamescdavis
Copy link
Member

@laurenbarker Oops, I keep forgetting there is a user relationship! Sorry about that!

jamescdavis
jamescdavis suggested changes Dec 15, 2017
View reviewed changes
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.

Please update user action relationship as well.

@adlius
Copy link
Collaborator Author

adlius commented Dec 15, 2017

A PR is coming up shortly.

@laurenbarker
Copy link
Contributor

Thank you!

@adlius
Copy link
Collaborator Author

adlius commented Dec 15, 2017

Rename completed. Since I touched the user model, I also took the liberty to add some tests.

jamescdavis
jamescdavis previously approved these changes Dec 15, 2017
View reviewed changes
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.

Thank you for updating the user/review-action relation and adding user model tests!

tests/unit/models/user-test.js Outdated
assert.equal(relationship.key, 'reviewActions');
assert.equal(relationship.type, 'review-action');
assert.equal(relationship.kind, 'hasMany');
});
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for adding all these!

@laurenbarker laurenbarker mentioned this pull request Dec 18, 2017
Merged
@laurenbarker
Copy link
Contributor

@adlius @jamescdavis, sorry for the flip flop. It looks like the user's review-actions relationship doesn't exist anymore so it can be removed from the model.

@adlius
Copy link
Collaborator Author

adlius commented Dec 19, 2017

@laurenbarker PR is updated to remove the review-action relationship for user and its respective model test.

@jamescdavis
Copy link
Member

Passing build: https://travis-ci.org/adlius/ember-osf/builds/319232973

This was referenced Dec 20, 2017
Closed
Merged
@jamescdavis
Copy link
Member

Still passing: https://travis-ci.org/adlius/ember-osf/builds/319331940

jamescdavis
jamescdavis approved these changes Dec 20, 2017
View reviewed changes
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.

👍

@jamescdavis jamescdavis merged commit 8329921 into CenterForOpenScience:develop Dec 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@laurenbarker laurenbarker laurenbarker requested changes

@jamescdavis jamescdavis jamescdavis approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@adlius @jamescdavis @laurenbarker