Closed (fixed)
Project:
Project issue tracking
Version:
7.x-2.x-dev
Component:
Issues
Priority:
Critical
Category:
Task
Assigned:
Reporter:
Created:
27 Apr 2012 at 20:10 UTC
Updated:
4 Jan 2014 at 02:05 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
webchickx
Comment #2
webchickHere were some comments from merlinofchaos to assist:
Comment #3
webchickA workaround for people testing in the meantime is to append the project shortname to the end of the URL like you do on drupal.org. E.g.
/node/add/project-issue/drupal
Comment #4
iamcarrico commentedWell, since the form is already setup to get the components, all that was needed was a simple #AJAX call--- it should all work from there.
Comment #5
iamcarrico commentedOne day I will get this right... renamed it ".patch" so testing can commence.
Comment #6
dwwThanks for taking a first stab at this! Glad it can all be done via Form API and #ajax support, that's a good sign.
Some problems upon visual inspection of the patch -- I haven't tested it yet:
A) This assumes there's a single node type called "project_issue". We need to be checking across all node types to know if they're configured to behave like issues. See the
project_issue_[node_type]variable introduced by chx at #1548222: node/add/project-issue/%project. Commit f34a537eb72f67b8f26cba4c1f68729246d6278a to be precise. ;)So, instead of a form_id-specific form alter, I think you're better off with the generic hook_form_alter(), and then inspect if it's a node form (I know there's an easy way to test for that, although I think it changed in D7 and I don't want to give you the wrong info, so just look in the form or search online for docs), and if so, check if the node type in question has the setting set. That's going to be a lot cleaner than either assuming a single node type (what you're doing here) or trying to provide id-specific alter callbacks for every node type that's configured to act like an issue.
B) This assumes that only the component is project-dependent. I guess that's technically true now, but we definitely want this to be able to support other project-specific fields. In particular, version and assigned. I guess that aspect can be moved into #1551248: Port project-specific "Assigned" functionality to D7 and #1328928: Refactor version selection box to leverage "version strings" instead of "release strings" respectively, but it'd be nice to write things here so that it's very easy and obvious to plug this same behavior in once those fields are properly treated as per-project magic.
C) project_issue_form_project_issue_node_form_alter() is missing a PHPDoc comment. The comment stub is there, but there's no content. ;) We try to keep everything documented in Project*, just like in core. Given A) this is probably going to become project_issue_form_alter(). So, you'd want something like:
You should make sure that wraps properly at 80 characters width, but otherwise, something like that.
Thanks!
-Derek
Comment #7
dwwAnother thought occurred to me about this -- I wonder if this is something that the dereference_list module should be doing itself for its own fields. That's what we're using for component. That won't help once we get to special-cases like version and assigned, but it'd work for component. And if this was solved "upstream" in dereference_list itself, then it should magically Just Work(tm) on any other fields like this people might add to their own issue node type(s).
Comment #8
iamcarrico commentedI will take a look at dereference_list and see if it an be better done there. Since that is the functionality, I tend to agree with you... its just a matter of getting the code right.
Comment #9
iamcarrico commentedOkay, so I did two things here... and two different patches to achieve the same effect. One is through the dereference field module, the other is the same as before (through the project_issue module), but with a few minor changes. The dereference version will automatically take any field created with the dereference module, and attach the proper ajax. I have not commented it out much yet, as I want to check the functionality first before making too many changes.
From what I can tell, the #ajax call cannot have more than one callback. I am still working on some other things, but it might be better for somebody who understands the JavaScript attached to this to take a closer look as I am really just relying on the PHP/API documentation on the JavaScript.
Comment #10
iamcarrico commentedOops, left a minor bit of dev code in there... he is the project_issues patch again, corrected.
Comment #11
senpai commentedAssigning this issue.
Comment #12
senpai commentedTagging.
Comment #13
dwwSorry this has languished. I was under the impression mikey_p was going to look at this, but sadly, he's been slammed and hasn't had a chance.
Meanwhile, is this still really an issue for project_issue or are we moving this to dereference_list? I'd certainly rather this was dereference_list's responsibility, if we can make that work. I'm too tired and sick (literally) to look at this closely right now, and I'm about to go out of town and offline for a few weeks. But if no one else has looked at this by the time I'm back online (around July 26th) I'll give it a careful review and test.
Thanks/sorry,
-Derek
Comment #14
senpai commentedTagging for Sprint 6.
Comment #15
iamcarrico commentedI just ran the Dereference list patch again and it seems to work. CoffeeIV will be testing it further to check code again.
-ChinggizKhan
Comment #16
rgristroph commentedI tested the patch for the dereference_list module with ChingizKhan and works when editting an existing project_issue node and not when creating a new one, we're working on that now, so setting this back to "needs work".
Comment #17
senpai commentedReassigning to @bdragon for a fix.
Comment #18
bdragon commentedWe can't really do better than a whole form reload, now that the fieldgroups are set up.
http://drupalcode.org/project/project_issue.git/commit/009a3584ea108c377...
Comment #19
dwwHrm... I just did a git pull and cleared all caches, but this doesn't seem to be working at all on my local test site. I see no change in behavior when trying to update the project on an issue, no rebuilding of the form, etc.
Furthermore, as I wrote in #13, I'd really rather this wasn't project_issue's problem, but instead was for dereference_list to sort out. However, maybe that's impossible given how intertangled things are.
Anyway, back to needs work since I'm not seeing any AJAX action at all.
Comment #20
dwwSorry, false alarm. There was some bizarre caching going on. It's working now.
I'd still like this to be dereference_list's problem, but that's not a d.o D7 launch blocker, and it's not going to work for our other special fields like assigned and version, anyway.
Comment #21
dwwp.s. I opened #1824498: Automatically replace the field element when the entity reference changes in the dereference_list issue queue for a more general solution, but that's not blocking the d.o D7 launch so I didn't tag it and it's not major or critical. What's here now is Good Enough For Launch(tm).
Thanks,
-Derek
Comment #22
iamcarrico commentedJust as a note, the two patches in #9 are not supposed to be used together. One was for dereference list to solve the issue, the other for project_issue. They both achieve the same thing independently, but just do it in different ways.
So, theoretically the dereference field patch can do everything you need. But there are limitations with the Drupal Form AJAX api for both.