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

Adds status field to the asset and changeStatus #144

Closed
wants to merge 2 commits into from

Conversation

gabelula
Copy link

@gabelula gabelula commented Dec 9, 2016

What does this PR do?

  • Adds status field to the Asset model
  • Adds changeStatus method to the Asset model
  • It changes the endpoint to change status to use that new method
  • Adds tests

How do I test this PR?

  • Check that the stream gets closed and open and that it works.

@wyattjoh wyattjoh closed this Dec 9, 2016
@wyattjoh wyattjoh deleted the bug-comment-stream branch December 9, 2016 15:06
Copy link
Contributor

@impronunciable impronunciable left a comment

Choose a reason for hiding this comment

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

looks good

@gabelula
Copy link
Author

gabelula commented Dec 9, 2016

This is still relevant as we do not have backend for the status.

@gabelula gabelula restored the bug-comment-stream branch December 9, 2016 21:54
@gabelula gabelula reopened this Dec 9, 2016
@wyattjoh
Copy link
Collaborator

wyattjoh commented Dec 9, 2016

The closed status of the asset is fully represented by the closedAt field, so i don't see the point of this as it seems like we are duplicating effort.

@wyattjoh
Copy link
Collaborator

wyattjoh commented Dec 9, 2016

See this PR #134 for the closed at field which provides the Opened/Closed status.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants