500585 - Merge issues, part 1

Status: RESOLVED FIXED

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

Description

[Wil Clouser [:clouserw]]

Alright, I've merged the webmocha branch to trunk (r25690:28598).  There were a total of 18 conflicts, most of which were easy to resolve.  A couple views might need tweaking.  I haven't been able to test it yet because the SQL is out of date and didn't merge.  Only two files didn't merge at all:

r28339 rewrote all of /tests/data/remora-test-data.sql rearranging the data and including the table structure.  I didn't merge any of that file.  Please add a patch only including whatever you added/changed to the file and not including any table structure - the file should be purely data.

r27398 rewrote all of /config/sql/remora.sql adding DOS line breaks.  There was a commit before and after that and I'm not sure what all needs to go into it without digging into each.  Please add a patch including only what changes need to go in and with unix line breaks.

I know I need to run /bin/run_once/tags2categories.sql and whatever you changed in remora.sql.  Is there anything else I need to run or be aware of?  Thanks.

Comment 1

[Wil Clouser [:clouserw]]

I read the diff - without being able to test yet, the only thing I saw was /site/app/controllers/developers_controller.php's _editAddonTags() has a comment that says "// Add tags here" but no code for adding the tags.

Comment 2

[Yem Huynh]

Attachment: patch for merge.

this patch contains a cleaned up version of remora.sql (without the DOS line breaks ) and also an updated tags2categories.sql which only contains commands to alter the existing schema to support Tags.

Comment 3

[Yem Huynh]

Attachment: remora sql diff.

this file contains all the triggers and new tables that make up the difference between the new Tags-related tables and the original remora.sql.

Comment 4

[Wil Clouser [:clouserw]]

Thanks Yem.  Merging now...

Comment 5

[Wil Clouser [:clouserw]]

Stuff left in this bug:

> r28339 rewrote all of /tests/data/remora-test-data.sql rearranging the data and
> including the table structure.  I didn't merge any of that file.  Please add a
> patch only including whatever you added/changed to the file and not including
> any table structure - the file should be purely data.


> /site/app/controllers/developers_controller.php's _editAddonTags() has a
> comment that says "// Add tags here" but no code for adding the tags.

Comment 6

[Wil Clouser [:clouserw]]

Any news on this?

Comment 7

[Patrick Sullivan]

Hey Will
For the latter part, that comment can be removed - I had thought the add action would happen in the developer controller but in fact I use the tags controller for that as well and redirect to the developer view. Sorry about the misleading comment. All that method needs to do is render the view.

Comment 8

[Wil Clouser [:clouserw]]

(In reply to comment #7)
> Hey Will
> For the latter part, that comment can be removed - I had thought the add action
> would happen in the developer controller but in fact I use the tags controller
> for that as well and redirect to the developer view. Sorry about the misleading
> comment. All that method needs to do is render the view.

Thanks Patrick - that helps.  It looks like tag adding is broken then because in webroot/js/tags.js there is a path hardcoded to /tags/add_ajax/ which is not always the case.  All paths should be relative or use the PHP constants/helpers to determine them.  In this case using AddonsHtmlHelper::url() or a similar function to pass the URL to the JS would work.  Can you do a quick check of the rest of the code to make sure paths aren't hardcoded as well?

Comment 9

[Patrick Sullivan]

Yes, I will go through the code and do that. I had the site in my web root so  missed that one. I'll have to figure out a way to get the base URL to the AJAX callback, as it's in its own .js file.

Comment 10

[Jeff Balogh (:jbalogh)]

My technique has been passing in urls as configuration parameters, example in webroot/js/amo2009/collections.js

Comment 11

[Mike Lee]

Work in progress. Aiming to complete it tomorrow.

Comment 12

[Patrick Sullivan]

Jeff, tht's cool - where do you pass those JS values in from the PHP?

Comment 13

[Patrick Sullivan]

Attachment: PAtch to fix hard coded URLs in ajax calls.

Comment 14

[Wil Clouser [:clouserw]]

r28893, thanks.  I filed bug 501132 for the remaining issue.

Comment 15

[Wil Clouser [:clouserw]]

Reopening.  JS-disabled case is still broken when adding tags on add-on detail pages.

Comment 16

[Patrick Sullivan]

Attachment: Patch to fix hard coded URLs in non-ajax calls.

Makes URLS relative to base path. Also re-adds tags_js_init.js which wasn't added to SVN with other changes from last patch.

Comment 17

[Wil Clouser [:clouserw]]

the patch included an extra file but I fixed it and committed r29169.  thanks.