Currently when you go to node/add/project-issue it's literally impossible to fill out the form, because the components list (which is required) has no data in it. This will also happen on edit when you try and switch an issue to another project.

We should AJAX-ify this form to solve both issues. So when you select a project, it should refresh the depedent fields (for now, just component, but in the future things like body, assigned to, version, help message).

Comments

webchick’s picture

Issue tags: +project, +drupal.org D7

x

webchick’s picture

Here were some comments from merlinofchaos to assist:

merlinofchaos
thinks.
Actually
If all you need to do is rebuild the whole form it may not be that difficult.

chx
rebuilding the whole form wfm
see project_issue_node_add on what's needed
biascally, $node->field_project[LANGUAGE_NONE][0]['target_id'] = $target_id; where $target_id is the chosen project

dww
well, ideally we don't destroy whatever you've already told the form. 

merlinofchaos
dww: Yes, an #ajax will submit all the values you have.

dww
nice


merlinofchaos
Well.
That probably fails on the autocomplete.
THough you might be able to do #ajax on the 'blur' event.
No, you'd want 'change'.
And potentially has to set the project id from post.

webchick
merlinofchaos: is there a good example that coul help someone ramp up to speed?

merlinofchaos
webchick: There's a mediocre but functional example in image field which uses pretty much the same technique to handle the change image button.
webchick’s picture

A 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

iamcarrico’s picture

Status: Active » Needs review
StatusFileSize
new1.04 KB

Well, 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.

iamcarrico’s picture

One day I will get this right... renamed it ".patch" so testing can commence.

dww’s picture

Status: Needs review » Needs work
Issue tags: +sprint 1

Thanks 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:

/**
 * Implements hook_form_alter().
 *
 * Checks if we're building a node form for a node type that is configured to be an
 * issue. If so, we inject a AJAX callback for when the project is changed so we can
 * update the dependent fields (e.g. component, assigned, version, etc).
 */
...

You should make sure that wraps properly at 80 characters width, but otherwise, something like that.

Thanks!
-Derek

dww’s picture

Another 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).

iamcarrico’s picture

I 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.

iamcarrico’s picture

Okay, 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.

iamcarrico’s picture

Status: Needs work » Needs review
StatusFileSize
new1.61 KB

Oops, left a minor bit of dev code in there... he is the project_issues patch again, corrected.

senpai’s picture

Assigned: Unassigned » iamcarrico

Assigning this issue.

senpai’s picture

Issue tags: +sprint 2

Tagging.

dww’s picture

Sorry 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

senpai’s picture

Issue tags: +sprint 3, +sprint 4, +sprint 5, +sprint 6

Tagging for Sprint 6.

iamcarrico’s picture

I just ran the Dereference list patch again and it seems to work. CoffeeIV will be testing it further to check code again.

-ChinggizKhan

rgristroph’s picture

Status: Needs review » Needs work

I 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".

senpai’s picture

Assigned: iamcarrico » bdragon
Priority: Major » Critical

Reassigning to @bdragon for a fix.

bdragon’s picture

Status: Needs work » Fixed

We 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...

dww’s picture

Status: Fixed » Needs work

Hrm... 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.

dww’s picture

Status: Needs work » Fixed

Sorry, 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.

dww’s picture

p.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

iamcarrico’s picture

Just 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.

Automatically closed -- issue fixed for 2 weeks with no activity.