Status

RESOLVED FIXED
10 years ago
3 years ago

People

(Reporter: mikelee, Assigned: yem.huynh)

Tracking

unspecified
5.0.7

Details

(Whiteboard: [webmocha])

Attachments

(1 attachment, 1 obsolete attachment)

2.70 KB, patch
jbalogh
: review+
Details | Diff | Splinter Review
(Reporter)

Description

10 years ago
With memcached on, I get the following (these are model issues):

User page:
- Tags do not update when a new tag is added via AJAX. When the addon
is reloaded, all new tags appear.
- When a tag is deleted and re-added via AJAX, it does not throw an
error like it used to.
- When a tag is deleted, it refreshes the tag list properly and all
added tags appear.

Developer tools page:
- Tags do not update when a new tag is added via AJAX. When the page
is reloaded, all new tags appear.

Admin tools page:
- No issues with memcached
(Reporter)

Updated

10 years ago
Whiteboard: [webmocha]
(Reporter)

Updated

10 years ago
Assignee: mikelee → yem.huynh
(Assignee)

Updated

10 years ago
Status: NEW → ASSIGNED
(Reporter)

Comment 1

10 years ago
Notes from Yem:

Need to follow up with Patrick on this issue:
- When a tag is deleted and re-added via AJAX, it does not throw an
error like it used to.

Work still in progress.
(Assignee)

Comment 2

10 years ago
got clarification from Patrick:

==========================
Yes, this wasn't very clear :)

Bad thing / unexpected : Tags do not update when a new tag is added via AJAX. When the addon is reloaded, all new tags appear.

Good thing:  When a tag is deleted and re-added via AJAX, it does not throw an error like it used to.

Good thing:  When a tag is deleted, it refreshes the tag list properly and all added tags appear.

Bad thing: Tags do not update when a new tag is added via AJAX. 
================================================================
So i will investigate into the ones marked 'Bad thing'.
(Assignee)

Comment 3

10 years ago
Posted patch bug fix (obsolete) — Splinter Review
tag list is now updated when adding tags (both as a dev and user ) due to fix on bug 501151 .  However what i found was that after a tag is added, refreshing the page makes the tag disappear.  I also noticed this happening on previews.  Attached is a fix but I'm not sure if it is the right thing to do.  However i do see this in other controllers where the beforeFilter() does something like this:

        // disable query caching so devcp changes are visible immediately
        foreach ($this->uses as $_model) {
            $this->$_model->caching = false;
        }

I think when adding the tag and the page is refreshed, the Addon is still cached, even though _add_tag() in tags_controller.php is calling markListForFlush()
(Assignee)

Updated

10 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Posted patch use cacheSplinter Review
Attachment #386653 - Flags: review?(jbalogh)
Attachment #386427 - Attachment is obsolete: true
Attachment #386427 - Flags: review-
Comment on attachment 386427 [details] [diff] [review]
bug fix

Removing caching on our main add-on display page sounds like a recipe for disaster
My patch uses the cache as intended but breaks inconsistently.  We're investigating that in bug 502144.  I'm reopening this as it's not fixed until a patch lands but there is no use working on this more until that bug is fixed.
Status: RESOLVED → REOPENED
Depends on: 502144
Resolution: FIXED → ---
Comment on attachment 386653 [details] [diff] [review]
use cache

Works for me now that we fixed cake, but it would be nice to have a comment about what's going on here:

>+        if (empty($addon_data)
>+            || $addon_data['Addon']['inactive'] == 1
>+            || !in_array($addon_data['Addon']['addontype_id'], array(ADDON_EXTENSION, ADDON_THEME, ADDON_DICT, ADDON_SEARCH, ADDON_LPAPP, ADDON_PLUGIN))
>+            || !in_array($addon_data['Addon']['status'], $valid_status)) {

Like /* Make sure we're showing good add-ons */
and /* here's why I didn't do this in SQL */
Attachment #386653 - Flags: review?(jbalogh) → review+
Thanks, I committed close to the same thing in r29331.  I'm pulling the contrib_details as well as array_unique()ing the $fields before we pass them off.
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.