Merge issues, part 1

RESOLVED FIXED in 5.0.7

Status

addons.mozilla.org Graveyard
Public Pages
--
blocker
RESOLVED FIXED
9 years ago
2 years ago

People

(Reporter: clouserw, Assigned: Patrick Sullivan)

Tracking

unspecified
5.0.7

Details

(Whiteboard: [webmocha])

Attachments

(4 attachments)

(Reporter)

Description

9 years ago
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.
(Reporter)

Comment 1

9 years ago
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

9 years ago
Created attachment 385312 [details] [diff] [review]
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

9 years ago
Created attachment 385313 [details]
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.
(Reporter)

Updated

9 years ago
Attachment #385313 - Attachment mime type: text/x-sql → text/plain
(Reporter)

Comment 4

9 years ago
Thanks Yem.  Merging now...
(Reporter)

Comment 5

9 years ago
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.
(Reporter)

Comment 6

9 years ago
Any news on this?
(Assignee)

Comment 7

9 years ago
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.
(Reporter)

Comment 8

9 years ago
(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?
(Assignee)

Comment 9

9 years ago
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.
My technique has been passing in urls as configuration parameters, example in webroot/js/amo2009/collections.js
(Reporter)

Updated

9 years ago
Whiteboard: [webmocha]

Comment 11

9 years ago
Work in progress. Aiming to complete it tomorrow.
(Assignee)

Comment 12

9 years ago
Jeff, tht's cool - where do you pass those JS values in from the PHP?
(Assignee)

Comment 13

9 years ago
Created attachment 385661 [details] [diff] [review]
PAtch to fix hard coded URLs in ajax calls.

Updated

9 years ago
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
(Reporter)

Comment 14

9 years ago
r28893, thanks.  I filed bug 501132 for the remaining issue.
(Reporter)

Comment 15

9 years ago
Reopening.  JS-disabled case is still broken when adding tags on add-on detail pages.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Updated

9 years ago
Assignee: mikelee → expatrick
(Assignee)

Comment 16

9 years ago
Created attachment 386175 [details] [diff] [review]
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.
(Reporter)

Comment 17

9 years ago
the patch included an extra file but I fixed it and committed r29169.  thanks.
Status: REOPENED → RESOLVED
Last Resolved: 9 years ago9 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.