501151 - Newly added tag is updated only after user refreshes page

Status: VERIFIED FIXED

(addons.mozilla.org Graveyard :: Public Pages, defect)

Description

[krupa raj[:krupa]]

steps to reproduce:
1.Log in to your AMO account.
2.Navigate to the details page for any add-on
3.click on "add a tag"
4.Enter "test" as a tag and click "Add"

expected result:
"test" is listed as one of the tags for that add-on

actual result:
After clicking add-on,the page reloads but the tag is missing.The tag can be seen by refreshing the page

screenshot:
http://screencast.com/t/LsCyzPQ9yS

Comment 1

[Wil Clouser [:clouserw]]

This works as expected on my dev copy, but not on preview.

Comment 2

[Patrick Sullivan]

I think this is a memcached issue. Yem is working on it now. What is the best way o do ths - invalidate the whole addon by adding it to the invalidate queue?

Comment 3

[Andrew Wooldridge]

Sending to Yem to investigate.

Comment 4

[Wil Clouser [:clouserw]]

(In reply to comment #2)
> I think this is a memcached issue. Yem is working on it now. What is the best
> way o do ths - invalidate the whole addon by adding it to the invalidate queue?

If that works, that's fine.

Comment 5

[Yem Huynh]

Attachment: bug fix

fixed. setting Addon->caching = false after tags are added and before findById() is called which will get the added tag too.  I see this done in a lot of controllers where write operations set the caching to false first before they do it so "changes are visible immediately".

Comment 6

[Yem Huynh]

also i want to add that you can't really invalidate the Addon in the model in this case..probably why other controllers use this caching = false mechanism too.  This is because to invalidate the addon in the cache you call $this->Cache->markListForFlush("addon:{$this->id}"); which will mark it for invalidation..but this invalidation won't happen until the end of the filter..but we need the updated addon right after the tag is added, which is before the end of the filter.

Comment 7

[Wil Clouser [:clouserw]]

I committed your patch in r29185 but I think disabling caching is a poor choice.  You've got the list of add-ons, you should be able to generate it without hitting the database again.  The entire function needs to be looked at with performance in mind though so I'll land this.

Comment 8

[krupa raj[:krupa]]

This looks fixed.Marking it verified for now.Will reopen if it surfaces