Closed (fixed)
Project:
User Relationships
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
6 Dec 2010 at 14:50 UTC
Updated:
8 Apr 2011 at 08:31 UTC
Jump to comment: Most recent file
Comments
Comment #1
alex.k commentedAdded a link to this issue to #671816: Drupal 7 port which is the main bet of a D7 port.
Comment #2
berdirWhat exactly is the "main module"? :)
UR doesn't have a module that matches the project name, can the update manager actually resolve this?
Comment #3
alex.k commentedThe "main" module is user_relationships_api. I think that the directory structure, and also the fact that many functions do not start with user_relationships_api_* come from the original 5.x version, and these issues were never corrected in the 6.x branch.
I am not sure we can rename the module to match the project name. We can, however, add a dummy module if that makes migration easier.
Comment #4
berdirOk, here is a patch.
- Moves user_relationships_api into the top-level folder
- Rename it to user_relationships
- Added an update function to user_relationships_ui, that automatically enabled user_relationships and also makes sure that it doesn't try to install the schema again
- Move the content of the api.inc into .module. There is no point in having separate files when you need to include them on every single page anyway.
I haven't moved over the admin UI yet, that should be possible, but this page is already huge. Will create a follow-up issue for that.
When you test this patch, make sure to run update.php before accessing the site directly. It *will* blow up if you don't.
Comment #5
mrf commentedGit apply gave me a couple of minor errors.
Looked over the patch and seems like there's not much happening but files moving and dependencies changing.
Applied the patch and spent a while clicking through the site, created some new relationship types, added some new relationships, and worked with some existing ones and I didn't come across any issues. I'd say this is ready to go.
I did get a warning from my user_relationship_limits module about the missing ur_api dependency, that will be up to module maintainers to fix but is worth noting.
Comment #7
berdirThanks for testing and reviewing.
Yes, I'm just moving stuff around, so that's why git apply shows some warnings.
BTW, the testbot is now enabled but obviously heavily confused by this patch. We'll see how well it goes after I've commited this. This will help prevent obvious bugs like the one that resulted in a PDOException when no relationship types were defined. It also encourages to write more tests :)
Comment #8
BenK commentedSubscribing
Comment #9
BenK commentedI tested the patch in #4 and ran update.php. Everything worked smoothly, no problems at all. So I think this ready to be committed.
--Ben
P.S. I know that you're not a big fan of the "UR" prefix in sub-module names. I don't care for that either. Do you want to change sub-module names to use "User Relationships" in a subsequent issue? We could also move over the admin UI in that same issue.
Comment #10
berdirThanks for testing, commited.
@BenK: Changing the visible module name has no side-effects, it is only used in the user interface. So we can change that any time we want, no reason to delay an alpha version because of that.