800341 - Basic Information section is not closed after changes are saved

Status: VERIFIED FIXED

(Marketplace Graveyard :: Developer Pages, defect, P3)

Description

[Cristian Boldan]

Steps to reproduce:
1. Load MP submissions page https://marketplace-dev.allizom.org/developers/submissions
2. Click "Edit Listing" link on a listed app
3. Click "Edit" button for the Basic Information
4. Make changes and click the "Save Changes" button

Expected results:
Changes are made and the Basic Information edit section is closed.

Actual results:
Changes are saved but the edit section does not close unless the Cancel link is pressed.

Notes/Issues:
Verified on FF18 (Win 7). Issue is not reproducing in production.
Screencast for this issue http://screencast.com/t/LbjYwfsTSOb5

Comment 1

[Christopher Van Wiemeersch [:cvan]]

I can't reproduce. In the future could you please list the URL in the bug? Thanks

Comment 2

[Christopher Van Wiemeersch [:cvan]]

On https://marketplace-dev.allizom.org/developers/app/appop/edit I can. Odd but not for other apps.

Comment 3

[Teodosia Pop]

We've seen this issue in our Selenium tests in Jenkins.

Application name: Mock Application 1351773174

Screenshot of the issue: http://qa-selenium.mv.mozilla.com:8080/job/marketplace.dev.developer_hub/165/HTML_Report/debug/tests_desktop_developer_hub_test_developer_hub_TestDeveloperHub/test_that_checks_editing_basic_info_for_a_free_app/screenshot.png

First time the issue appeared in the tests: http://qa-selenium.mv.mozilla.com:8080/job/marketplace.dev.developer_hub/163/testReport/

Comment 4

[[:AndreiH]]

This is still happening on dev and also the changes are not saved after refreshing the page.

Comment 5

[Iulian Timis]

Issue is still reproducing in both -dev and production on FF24 (Win 7).
Please see screencast http://screencast.com/t/bm86WVHK8

Comment 6

[[:AndreiH]]

Our automation test is passing, this seems to be fixed.

Comment 8

[Vaidik Kapoor [:vaidik]]

Our automated tests have caught this again and I manually verified this.

Here is the URL I tried: https://marketplace-dev.allizom.org/developers/app/adealor-yleri-aclivtournban/edit

Screenshot: http://imgur.com/8CtYiLA

Comment 9

[[:AndreiH]]

I can see this behaviour happening on the newest app submitted. On older apps the behaviour is not reproducible

Comment 10

[Mathieu Pillard [:mat]]

This is now happening... for every packaged app, it seems.

The basic info form is not validating because it's missing manifest_url, but because that field is not displayed for packaged apps it's not showing up as an error. This means any changes to the description and to the app slug won't be saved. Categories work because the code is separated from the main validation checks.

Comment 11

[Mathieu Pillard [:mat]]

Amending the comment above: description and slug are saved, because the code for the categories saves the (modified) addon instance. But this means we might be able to bypass checks done in the clean_() methods of the form, and that these changes are not logged. Downgrading priority, but we should definitely fix this.

Also been looking in the past and it seems to have been broken for ages because of the same root cause.

Comment 12

[Vaidik Kapoor [:vaidik]]

I can confirm that this is failing only for packaged apps. Our test is xpassing now because it tries to edit details for a hosted app. But when I manually try to edit a packaged app, I still encounter this issue.

Comment 13

[Mathieu Pillard [:mat]]

I'll push the fix for this tomorrow, it'll be going in production next week.

Comment 14

[Mathieu Pillard [:mat]]

Fixed: https://github.com/mozilla/zamboni/commit/5736c964f9c0fa1307bce6092e7573be3779ad33

See description for steps to reproduce on -dev, use a packaged app and make sure changes in that form work, including categories change.

Comment 15

[[:AndreiH]]

This is now Verified and Fixed. Thanks!