Cannot remove an app from a collection from rocketfuel

VERIFIED FIXED in 2013-12-17

Status

P2
normal
VERIFIED FIXED
5 years ago
5 years ago

People

(Reporter: cvan, Assigned: basta)

Tracking

2013-12-17
Points:
---

Details

(Whiteboard: [incorrect_implementation], URL)

(Reporter)

Description

5 years ago
STR:
1. Go to a collection.
2. Click the "X" for an app to delete it.
3. Notice JS error:
Uncaught TypeError: Cannot read property 'id' of undefined
http://f.cl.ly/items/0Y020e3O3L3T2p0D2x2w/Screen%20Shot%202013-12-17%20at%2012.08.08%20PM.png

NOTE: Reproduce in prod, stage, and -dev:
https://marketplace.firefox.com/curation/collection/games-col-featured
https://marketplace.allizom.org/curation/collection/featured-apps
https://marketplace-dev.allizom.org/curation/collection/wowowowo

-

This fixes the problem somewhat such that the model cache look up works, but then I see a "[Object][object] was deleted" notification:

diff --git a/src/media/js/views/collection.js b/src/media/js/views/collection.js
index 22d8508..921bee8 100644
--- a/src/media/js/views/collection.js
+++ b/src/media/js/views/collection.js
@@ -185,7 +185,7 @@ define('views/collection',
         $app.hide();
         var app_id = $app.data('id');
         var collection = get_collection();
-        var app = app_model.lookup(app_id + '', 'id');
+        var app = app_model.lookup(app_id, 'id');
 
         requests.post(
             urls.api.url('remove_app', [collection.id]),
@@ -212,7 +212,7 @@ define('views/collection',
         var app = $this.data('id');
         $this.parent().remove();
         var collection = get_collection();
-        var app_data = app_model.lookup(app + '', 'id');
+        var app_data = app_model.lookup(app, 'id');
         var $app = $(nunjucks.env.render(
             'helpers/app.html',
             {'this': app_data, 'allow_delete': true, 'allow_reorder': true, 'tag': 'li'})

I clearly don't know what I'm doing.
(Reporter)

Updated

5 years ago
Summary: Cannot delete an app from rocketfuel → Cannot remove an app from a collection from rocketfuel
(Reporter)

Updated

5 years ago
Depends on: 951353
(Assignee)

Comment 1

5 years ago
The [Object object] is because the API now returns locale objects; we need to run that through `utils.translate()`.

It looks to me like there's a deeper issue. I'll look at the API, but what it seems like is the API is now returning a number for the ID rather than a string containing a number. If that's the case, then the fix in comment #0 should be fine to fix the lookup (I think). I'll investigate further.
This one is on the API.

I made the mistake of attempting to JSON-decode data in a branch that got run regardless of what the Content-Type header was set to, so it's flailing on Rocketfuel's application/x-www-form-urlencoded.

PR coming shortly.
Sorry, I'm extra dumb today. I conflated this with bug 951353. Carry on...
(Assignee)

Comment 4

5 years ago
Upon further investigation, the fix in comment #0 (despite being completely non-obvious) is the correct fix. IIRC, we might even be able to remove the second parameter ('id'), since that's the primary key for the collections model cache, anyway. I could be wrong. We may never know.
(Assignee)

Comment 5

5 years ago
https://github.com/mozilla/rocketfuel/commit/c0382dd174893a9d0de926ad07805a0513164a35
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Reporter)

Updated

5 years ago
Assignee: nobody → mattbasta
Target Milestone: --- → 2013-12-17

Comment 6

5 years ago
verified fixed @ https://marketplace-dev.allizom.org/curation/collection/new-collection
Status: RESOLVED → VERIFIED

Updated

5 years ago
Whiteboard: [incorrect_implementation]
You need to log in before you can comment on or make changes to this bug.