Tumblr posts should be deleted when users delete their originating Diaspora post.

[fabacab]

This implements cascading deletion for Tumblr posts created through Diaspora's Tumblr service. (I realize there's no spec file attached; I don't know much about them, and SOMEONE is encouraging me to send a pull request early….) ;)

[jhass]

Just change all accesses to clients instead.

[jhass]

Okay, this is looking pretty good by now, two things:

1. I'd like at least a spec that it hits the Tumblr API when deleting, have a look at the spec file for the facebook service to get an inspiration.
2. We need to clean this branch:

git remote add upstream git://github.com/diaspora/diaspora.git # if not done already
git fetch upstream
git branch -m develop develop-bak
git checkout -b develop upstream/develop
git branch -m tumblr-delete-deleted tumblr-delete-deleted-bak
git checkout -b tumblr-delete-deleted upstream/develop
git cherry-pick e52e48820a169aa6ace1412896f33027ffef1ebe
git cherry-pick 695be1d4391ea3fe3338e09816d7ca14e5278d59
git rebase -i upstream/develop # Select squash for the second commit
git push -uf origin tumblr-delete-deleted

[fabacab]

@MrZyx, I don't really understand what "I'd like at least a spec that it hits the Tumblr API when deleting" means. I spent a few hours this afternoon trying to make sense of RSpec and the services' spec files, but couldn't get very far. I understand the general principles here but do not yet have enough working knowledge of things to make these things "go."

FYI, I have been trying things like:

  describe '#delete_post' do
    it 'removes posts from tumblr' do
      stub_reqest(:post, "http://api.tumblr.com/v2/blog/foodbar.tumblr.com/post/delete").
        to_return(:status => 200)

      @service.delete_post(@post)
    end
  end

This seems to work, as it adds an example that passes, but I'm honestly not confident I'm testing the behavior of the delete_post method correctly. Moreover, my changes to the post method seem to be causing the posts a status message to tumblr example to fail, but I can't actually figure out how to make that test pass because, again, I don't think I know what I'm doing.

So, I just committed what I've got now. If you could take another look and wouldn't mind explaining WTF is happening here, I'd appreciate it. (And yes, I googled a lot of things, but all the articles I'm finding assume knowledge I just don't have, which makes them worse than useless, it makes those articles both useless and frustrating to read.)

Also FWIW, it would have been helpful to me to be given a link to https://wiki.diasporafoundation.org/Testing_Workflow as this describes some basics I needed to know. I spent a little while Googling until I came up with this. Not a huge deal but if you're open to some criticism, please consider sending this URL to others who describe themselves as new or unsure about Diaspora's rspec stuff.

[jhass]

Sure, I'll keep that in mind, thanks.

The spec actually looks good, let me explain why: We have gem in called webmock, which catches all calls to external sites. It raises an error if it sees one that has not previously been registered. With the stub_request method you do register that call, so effectively that's an expectation. I'm not sure if it would fail if the request doesn't come, but since the other tests have the same behavior that would be something to assure at a different time and is out of scope of this PR.

The other spec is failing because we now depend on the response of the post request, so we have to return one in the spec. I also added an assertion that the returned ids are stored:

    it 'posts a status message to tumblr and saves the returned ids' do
      response = mock(body: '{"response": {"user": {"blogs": [{"url": "http://foo.tumblr.com"}]}}}')
      OAuth::AccessToken.any_instance.should_receive(:get)
        .with("/v2/user/info")
        .and_return(response)

      response = mock(code: "201", body: '{"response": {"id": "bla"}}')
      OAuth::AccessToken.any_instance.should_receive(:post)
        .with("/v2/blog/foo.tumblr.com/post", @service.build_tumblr_post(@post, ''))
        .and_return(response)

      @post.should_receive(:tumblr_ids=).with({"foo.tumblr.com" => "bla"}.to_json)

      @service.post(@post)
    end

Btw. are you sure resp.code returns a String?

[fabacab]

Huh. Okay, thanks for that explanation, @MrZyx. So maybe what's tripping me up is the fact that the domain-specific language is not what I'm familiar with. I understand assertions but am having trouble mapping what I know to the expressiveness in the RSpec DSL. Comparing your snippet here to the previous spec helps me understand what's happening, and I appreciate the hint about WebMock. (Now I can google that and more-or-less grok what the articles I'm finding are talking about.)

So, am I to understand correctly that I should update the tumblr_spec.rb file with the snippet you provided here? And that the removes posts from tumblr method is actually doing the right thing by registering the HTTP POST request and then expecting the correct result? If so, yay!

Also, I'm pretty sure resp.code is a String. As far as I can tell, resp is a Net::HTTPCreated object, which inherits its code property from Net::HTTPResponse, which the docs describe as a string. And when I puts it, the ./script/server output says, "201". Sooooo…yes?

[jhass]

Yep and yep to your questions.

If you want to get 200% sure about the code use the p method to print it, it should print some quotes if so, with puts you see no difference between 201 and "201 "

[fabacab]

Okay, @MrZyx, I've pushed the latest ("cleaned"/squashed) commit tonight, as you instructed.

As an aside, is there any reason why squashing and then force-updating topic branches this way is "better" than just, y'know, taking all the commits themselves? It seems like this would make the development history harder to look through, to me.

(Oh, and yes, resp.code is most definitely a String. At least, according to p.)

[jhass]

No, having one self contained commit for each issue makes the history actually cleaner, this way you can git blame something and get all associated changes without searching for the corresponding merge commit.

[jhass]

Thank you very much!