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

Set modified_from to "code_corps" on tasks and comments when updating from codecorps #1163

Merged
merged 1 commit into from
Nov 7, 2017

Conversation

begedin
Copy link
Contributor

@begedin begedin commented Nov 7, 2017

What's in this PR?

This PR makes it so that every time a task or comment is modified from the client application, thus, from CodeCorps, not some other location, the :modified_from field on that Task or Comment respectively is set to "code_corps".

"code_corps" was already our field default, so I wen't with that, instead of "codecorps" as instructed in the issue.

Since it was our field default, the field was set correctly upon initial creation from the client app. However, a subsequent modification from github, followed by modification from codecorps, would result in :modified_from being incorrectly stuck on "github", even though the last source of the modification was in fact the client app.

Note that the create_changeset is also used in our seeds. It makes sense as it is, but if we want to seed our db with tasks or comments linked to github, we need to take that into account.

References

Fixes #1145

{:ok, updated_comment} = comment |> Comment.Service.update(@update_attrs)

assert updated_comment.modified_from == "code_corps"
end
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A changeset test might seem enough, but I wanted to add a sort of "integration" test to. The service is where the modification gets commited to the db.

test "sets task :modified_from to 'github'" do
%{github_issue: github_issue, user: user} = setup_test_data()
{:ok, tasks} = github_issue |> IssueTaskSyncer.sync_all(user)
assert tasks |> Enum.all?(fn task -> task.modified_from == "github" end)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the same reason as the service test above, this is a sort of an integration test, even though our sync changeset already has the needed assertions in its own tests.

I did not do the same for comments because of the complexity of the test data there. I'm hoping I'll get a chance to decouple the comment syncer from payloads first.

@joshsmith joshsmith merged commit 5bd21f8 into develop Nov 7, 2017
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.

None yet

2 participants