Display parts of the issue edit form instead of the comment form on issue pages

If the form is to contain only a selection of fields from the issue node rather than all fields, then the code I wrote for https://drupal.org/project/multiple_entity_form could be useful to copy from.

Are the changes to fieldset styles part of the design, or an artifact of wireframing? I'm assuming they are part of the design, and should be applied everywhere we have fieldsets on Drupal.org.

Last comment was meant for the parent issue. Ignore it here. This issue shouldn't touch fieldset styling, that's a separate issue.

This all looks okay. The new block should be in the drupalorg_project module.

I expect we can use the regular call to drupal_get_form($node->type . '_node_form', $node) with either an extra argument, or setting a property in $node to trigger the extra altering in drupalorg_project_form_project_issue_node_form_alter().

@drumm It is not an artefact, it is part of the design.

> Create a block to display fields from the issue edit form on the issue page

Wouldn't it be better to do this with hook_node_view(), as that's the same way that comment module adds the comment form to the node?

Another thing, should this really be in the d.org customizations project?

If it's a DX improvement/fix to use of project issues on d.org, the presumably it's also a DX improvement to the Project Issue module. And it makes sense to have as few customizations to the site as possible and put functionality in contrib modules where possible.

I would agree with #8. As the #1 users of project_issue, if it's not working for our community, I'm sure it won't work for others'.

Here's some very VERY rough work in progress. Patch is on project_issue.

I definitely think hook_node_view() is the way to go. A standalone block needs to be leveraged in with something like Context or block settings that can't be exported; hook_node_view() does the work for us. Unless there are reasons for needing a block that I've missed?

I tried drupal_get_form($node->type . '_node_form', $node) and I'm not keen. The node form has a ton of stuff that we probably don't need here -- such as all the things that go in vertical tabs. I think it's easier to have a custom form to which we attach the fields we want that have to doctor the node form to remove the bits we don't want.

Here is patch which currently on page-drupal.redesign.devdrupal.org

I think we have 2 options for displaying issue form:
1. As block hook_block_info()
2. In hook_node_view()

Actually in this patch not working revisions, i.e. after submit form revisions improperly stored.
The next issue it's hide already uploaded files. @wechick found bug, what after upload new file old files were deleted. Code for save old files is in custom submit callback drupalorg_project_issue_form_submit, but it's not always works properly, need to improved.

I'd really want to see how this actually looks in action, but after logging in with drupal/drupal my user credentials don't work. Is access intentionally limited to only a few or am I doing something wrong?

...and is it @ form-drupal.redesign.devdrupal.org or page-drupal.redesign.devdrupal.org

@klonos: best to ask about that on the issue / group post about the prototype itself.

Tried patch 11.

I can add comments and change properties. A few problems I found:

- this adds a dependency on field_group module
- node subject form element is at the bottom of the form
- instead of hook_form_alter() this should use hook_form_FORM_ID_alter() (twice).

I also think that whichever approach we take for this, we need tests!

Ok, first order of business is to reconcile the two different stabs at solutions we have in this issue.

sanchiz's patch (which is up on the staging site at http://page-drupal.redesign.devdrupal.org/) takes the "hook_form_alter() + embed the node form" approach. It starts by hiding all fields on the comment form (by setting #access to FALSE), and then embeds the node form in its place, hiding various fields on that.

Pictures are 1,000 words and all that, so what it does is change this:

To this:

Then embeds the node form, which makes the form look like this ungodly horror-monster:

Then hook_form_alter()s again to remove a bunch of stuff to make it look nice and lovely like the final result:

That does indeed seem like an awful lot of work we have to both do, and then undo. :( However, by keeping the same form structure, form IDs, etc. we are better assured that various submit()/validate() stuff will run when those buttons are pressed. It may be that it's less code in the long run than joachim's approach.

Speaking of which, joachim's approach is the "let's start at the mockup and build a custom form tailored specifically to it, then do the node edit logic needed afterwards." So we avoid undoing work we've already done, and instead start afresh. Though joachim's patch is not really viable, since there's not even a save button. :) It's simply laying out the general recommended architectural approach.

We're kind of damned if we do, damned if we don't, though. Either approach is going to mean things like adjustments to settings in the Field UI, etc. won't take immediate effect, and the display code will need to be tweaked accordingly. I wonder if we could use something akin to D8's "form modes" in D7, where we could set up an alternate display of the node edit form for "comment-like" node edit form, pulling in only the fields we want, and avoid all of this custom coding. Hm. Display suite?

Anyway, since sanchiz got pretty far with his approach (the form is saving and posting comments and so on, although there are bugs), it's probably the one to go with. I might spend some time with joachim's approach and see if it can be made viable, else a totally module-based thing. Then at least we'd have a couple of solutions to pick from.

@webchick agreed. My code was written for prototype asap for working and yet very far from the final result.

Oh, sorry, sanchiz! I *really* wasn't trying to slag your code, at all. You're doing a completely valid approach. And it's even working! :D I'm just musing about how best to balance "Drupal.org needs this ASAP" with "We ideally want Project module to be used by people other than Drupal.org" with "We want to avoid maintaining custom code for Drupal.org whenever we can."

Your code is still the only viable approach so that's the one we should build from unless something more complete comes along.

Just to rule out Inline Entity Form... what that would do is stick a form on the comment form allowing you to create a *new* issue, but what we want to do instead is edit the *existing* issue. (Though I suppose we might be able to do some fancy tricks w/ hook_form_alter() to put the existing issue's data in there.)

Notes: I implemented this by going to admin/structure/types/manage/project-issue/comment/fields (Issue comment fields) and adding an "Issue" Entity Reference field, with a widget of "Inline entity form - Single value," filtered to "Issue" type.

Hahaha, well I completely borked my local install playing with this (I accidentally deleted something I shouldn't have :)) so I need to rebuild that now, but it's worth pointing out... Drupal.org already has Fieldgroup module installed, and it turns the Manage Fields screen into this:

Which sure makes it look like a lot of the work here (renaming/consolidating fieldsets, re-ordering fields, etc.) could just be clicked together in the UI with zero lines of code necessary? I wonder if this has already been tried and didn't work out for some reason?

Provided the fieldsets on the node create/edit form are same as on the comment-ish update form, then yes, the UI admin approach is viable.

> We're kind of damned if we do, damned if we don't, though. Either approach is going to mean things like adjustments to settings in the Field UI, etc. won't take immediate effect, and the display code will need to be tweaked accordingly. I wonder if we could use something akin to D8's "form modes" in D7, where we could set up an alternate display of the node edit form for "comment-like" node edit form, pulling in only the fields we want, and avoid all of this custom coding. Hm. Display suite?

Super summing up in #14 :)

Display Suite does have a submodule which says 'Manage the layout of forms in Display Suite.'. I can investigate it, but I'm concerned that DS is a fairly heavy module, and d.org is (I gather?) already struggling with searches.

> I might spend some time with joachim's approach and see if it can be made viable, else a totally module-based thing.

I'm happy to push on with my approach some more if you think it's useful to do so so we can better compare the two approaches. One thing I realized since posting the patch is that as it's totally separate from the node form, we'd have to rewrite the preview functionality if we wanted it. Which will add to its cost.

Just tried DS Forms. It isn't going to do what we want. It doesn't allow alternate displays of entity forms, just lets you use DS features on the layout of the form.

Yeah, I also played with that but it seems like the wrong tool for the job; it'd probably be good if we wanted to lay the entire form out in two columns or so, but not really for creating a partial form.

I definitely can't weigh in here on technical merits of one approach vs. the other, so I can't say definitively "sure, please continue pushing on your approach!" because I've literally no idea if it'll be accepted or not. :) I tried pinging dww on IRC to weigh in here.

However, if you want to spend another couple of hours seeing how far it gets you, I definitely wouldn't disagree. Although my "gut" tells me that due to all of the logic typically embedded in _submit() and _validate() handlers in a typical module (I've not looked under the hook in project_issue so no idea if that applies here), hook_form_alter() is probably going to be the safest/easiest route. But it'd be interesting to see for sure, although I have to be upfront and say it may be wasted effort.

I personally still want to try pushing on the "can we do this via configuration and not much code" route for awhile longer and see where that ends up, since logically to me that seems the best if we can somehow pull it off.

Ok, just spent some "Quality Time™" with markcarver talking through this issue. Since he, unlike me, actually builds sites for a living, I'm inclined to believe him when he says that there's nothing like D8 form modes in D7 right now, especially since my couple of hours of research backs this up. :\ So I think the awesome configurable yay way is out, at least for now.

That leaves the choice of two types of custom code, and one of them has a mostly-working solution that we can just take the rest of the way there. So since this issue is causing immense pain in every single interaction we have with one another on the issue queue, we should just solve it the most direct route and then clean it up after. That means going back to D.o customizations and going with sanchiz's form_alter() approach. In a later iteration, we could clean the code up further going a different implementation route, including whatever's needed to genericize it enough to go into project_issue. (Maybe we could spin off a sub-issue to discuss the best way to go about this.)

So flipping this issue back to its original spot. I also noticed today when we were setting up the Git repo that page-drupal has different code on it than what sanchiz uploaded in #11, so going to upload that in addition. (I'm sure it's his code, though, just a bit further than #11.)

Here's that patch. (No commit credit for me; this is all sanchiz I believe.) Note that this includes more than just the drupalorg customization stuff, so needs to be refined down further.

sanchiz, please ping markcarver to get access to the github repo where we're working! :)

Ok, things i meant to get to tonight but didn't because I totally ran out of steam (one too many 5am mornings the past week):

- Most of the logic actually should live in drupalorg_project rather than drupalorg, given the nature of the changes.
- Per joachim, separate hook_form_FORM_ID_alter() hooks are preferred for legibility.
- The update function is duplicated twice, and those numbers conflict with some other updates on newer dev servers. Fixed in master by removing one and renaming it to 7019. (though, does this still belong in project_issue.install? probably drupalorg_project.install now?)

+++ sites/all/modules/drupalorg/drupalorg/drupalorg.module	2013-12-16 13:14:56 +0000
@@ -1121,6 +1121,62 @@
+    if (!isset($form['#node_block_form'])) {
+      $form['#node_block_form'] = FALSE;
+    }

That seems to be dead code; we're using $node->embed_issue_form above/below.

+++ sites/all/modules/drupalorg/drupalorg/drupalorg.module	2013-12-16 13:14:56 +0000
@@ -1121,6 +1121,62 @@
+  if ($form_id == 'comment_node_project_issue_form' && isset($form['#node'])) {
+    $node = node_load($form['#node']->nid);
+    if (project_issue_node_type_is_issue($node->type)) {

I also find this confusing... we're already in comment_node_project_issue_form... do we need the second check to make sure we're really really in an issue form?

Just want to document a conversation about using the same technique in form_process_vertical_tabs() to fix the AJAX POSTing issue.

[12:35 AM]  markcarver:	 webchick: oh, btw... I was thinking about the AJAX POSTing issue which causes the fieldsets to re-collapse and blurs focus on the active element
[12:36 AM]  webchick:	 yeah. any ideas?
[12:36 AM]  markcarver:	 vertical tabs actually has a pretty ingenious solution by setting a hidden field with the value of the "active" tab
[12:36 AM]  webchick:	 hmmm
[12:37 AM]  markcarver:	 we could do something similar with fieldset states/focused fields
[12:38 AM] markcarver	 is searching for the function
[12:39 AM]  markcarver:	 webchick: https://api.drupal.org/api/drupal/includes!form.inc/function/form_process_vertical_tabs/7
[12:39 AM]  webchick:	 nifty
[12:40 AM]  webchick:	 yeah that definitely looks worth a shot
[12:40 AM]  markcarver:	 I can do a bit of JS to detect the hidden field(s) and then set the fieldsets/focus accordingly
[12:41 AM]  webchick:	 just make sure to comment the heck out of that code. :D

Ok, here's the current state of things. I've not yet deployed this to the dev server because I'm hitting some weird problems w/ files on localhost and want to sort those out first. (Probably because of the "files" symlink.)

This addresses the stuff I brought up in #24, plus adds some comments that document what I figured out by commenting out certain parts and seeing what broke. The patch also includes markcarver's work in #2161601: Create styling that matches issue edit form mockup.

Ahem.

I think that the Issue summary and the Issue relations should be in separate fieldsets. One might want to update either one of these without updating the other. In such cases, we should keep the UI as minimal as possible. Having these in separate fieldsets does exactly that.

Once again. This issue is for implementation. Not to discuss the design. If you want to discuss the design, that discussion happens in https://groups.drupal.org/node/390268.

Sorry to be a hard-ass about scope, but the sooner we get this deployed, the sooner every single interaction we have on Drupal.org will be far more pleasant. We can always refine things further in the future.

Yes Angie, I got it the first time, but sorry that discussion (as any discussion in d.o in the form of a post) is not much of a use to me unless I can follow posts the same way we can follow issues (most importantly be notified about new comments so I don't miss any). I need to be notified about new comments in some way like https://drupal.org/project/issues/user because I don't use email notifications.

Now, I know I'm making it worse by adding noise to this issue (and risking your wrath), but the problem I explain was raised many times in the past and a lot of people that feel the same way about that suggested that we always open issues instead of starting discussions. Either that or #1304216: A user should be able to "follow" individual pages of content and receive email notifications for new comments

Anyways, since this is pointless, I went ahead and opened #2162665: Place the 'Issue summary' and the 'Issue relations' part of the issue edit form in separate fieldsets.. I'm adding it as a child issue but stressing in its issue summary that it should neither block nor slow down implementation of this issue here or #2159409: [Meta] Implement first iteration to improve issue queue workflow.

Instead of shoehorning the edit form into the comment form, can it be shoehorned into drupalorg_project_node_view()? Then we can turn off the comment form with the usual "Show reply form on the same page as comments" setting.

Thanks for cleaning up all the hacks for #2097927: Remove 'Update this issue' link next to main comment form when issue updaters are used to the new green button.

(I'd rather trade one form rendering for another, rather than hide one and add another.)

Well that makes a lot of sense!

Also, over at #2159409-34: [Meta] Implement first iteration to improve issue queue workflow, dww indicated that he'd rather make the UI/UX of the two forms more similar than different (among other things), so it probably makes sense to do most of this altering (fieldsets, etc.) for the issue edit form period, and only doing "embed-specific" logic (removing all the vertical tabs, file listing) where necessary.

I tried to roll a patch for this, but when I do this:

      $issue_form = drupal_get_form('project_issue_node_form', $node);
      $node->content['issue_form'] = render($issue_form);

...in drupalorg_project_node_view() I'm getting errors like this:

Warning: Invalid argument supplied for foreach() in element_children() (line 6396 of /Users/webchick/Sites/deedotoh/includes/common.inc).
Warning: Illegal string offset '#children' in drupal_render() (line 5863 of /Users/webchick/Sites/deedotoh/includes/common.inc).
Warning: Illegal string offset '#children' in drupal_render() (line 5873 of /Users/webchick/Sites/deedotoh/includes/common.inc).
Warning: Illegal string offset '#children' in drupal_render() (line 5911 of /Users/webchick/Sites/deedotoh/includes/common.inc).
Warning: Illegal string offset '#printed' in drupal_render() (line 5918 of /Users/webchick/Sites/deedotoh/includes/common.inc)

Way too tired to debug common.inc this evening. :P

One other note about this approach... it would mean we need to do a migration job to take all existing issues and turn off their comment forms. Unfortunately setting it at the node type level only stops the comment form for new issues.

However, it should be pretty easy. Basically UPDATE node SET comment = 1 WHERE type = 'project_issue';

Ok, japerry and DyanneNova and I had a hackfest tonight on this at Chez Webchïck, and as a result I'm almost 100% happy with how this works! :D

1) Per dww, it makes the fieldset/fieldgroup changes to both the bigger node edit form and the embedded node form. We default issue metadata to open on full edit, collapsed on embedded edit.

2) We did not go the route of setting "Comment setting -> Closed" on all nodes that drumm suggested in #31. Not only to avoid the needless migration path (as well as severe divergence from upstream project_issue), but also to avoid access denied issues associated with posting comments to nodes that are closed.

3) Instead, we did a clever hack (100% of credit goes to japerry) of using hook_node_view() and simply setting the $node->content array index to he same value as what comment module would normally use. This eliminates the duplicate form rendering, and also makes the position of the form in the proper place. It also seems to totally eliminate weird bugs I was seeing before with components mysteriously going missing randomly.

4) Cleaned up some comments / implementation of various spots where I'd left @todos before.

Remaining work:

- We should figure out how much of this we want to move into project_issue "proper" (e.g. the fieldset grouping/re-labelling, the embedded node form itself)
- We need the nice fieldset stylings on the embedded form to appear on the main node form as well.
- I still think we should fix the issue I found earlier with the files fieldset re-collapsing after uploading a file. It's a pretty nasty UX problem. Emilie spent some time looking into this, I'll have her follow-up.
- BUEditor is not showing up for the "Dries" user on localhost/dev. We're trying to get to the bottom of that.

This patch is now deployed on http://form-drupal.redesign.devdrupal.org/. Please test! (drupal/drupal, then bacon/bacon)

Spun off #2164105: Don't collapse the files fieldset when uploading new files (part of issue queue re-redesign) about that the file uploading issue.

@webchick great! All works for me.
But noticed one thing, Revision comparison not working for all issues.
Example: http://form-drupal.redesign.devdrupal.org/node/2158473/revisions/view/67...

I spent much time for understanding why this happens and how it works, but still not have result.

Oh, great catch. At a glance, I'd say we need another entry in drupalorg_project_menu_alter() to pull in node.pages.inc on node/%/revisions. I won't have a chance to look until later today, though.

Btw, if you haven't yet, sanchiz, ping markcarver on IRC/email for access to the Github repo. Would love to have you involved since you authored the original working patch! :D

Heh, unexpected baby nap == looking at this sooner than expected. :)

Short answer: We need a different way of including node.pages.inc whenever the form is displayed or could possibly be displayed, so enter the form_load_include(). (Kudos to torotil for posting their solution at https://drupal.org/node/1454634 which totally worked!)

So I removed the hook_menu_alter() bits and replaced them with this:

+      // Make sure the form builder, validation, etc. functions are available
+      // since we're displaying the form at a non-standard URL.
+      $form_state['build_info']['args'] = array($node);
+      form_load_include($form_state, 'inc', 'node', 'node.pages');
+
       // Render the issue edit form in place of the comment form.
       // This works by re-using the same array indexes as the comment form.
       // Magic! :)
-      $issue_form = drupal_get_form('project_issue_node_form', $node);
+      $issue_form = drupal_get_form('project_issue_node_form', $node, $form_state);
       $node->content['comments']['comment_form'] = $issue_form;
     }

Seems to work fine. Patch attached, but I don't have permissions to update the dev site for some reason. :(

@webchick,

- BUEditor is not showing up for the "Dries" user on localhost/dev. We're trying to get to the bottom of that.

This is just problem with images. Images for BUEditor placed at sites/all/libraries/bueditor, but in the form-drupal.redesign.devdrupal.org repo and resp. local repo this folder does not exist.

I've pulled in the changes to the form dev environment for ya'll to see.

Re: BUEditor icons:

This is just problem with images. Images for BUEditor placed at sites/all/libraries/bueditor, but in the form-drupal.redesign.devdrupal.org repo and resp. local repo this folder does not exist.

I do not believe this is the problem. If you login with any other user (ie bacon/bacon), they see the form just fine. Icons are located at: http://form-drupal.redesign.devdrupal.org/sites/all/modules/bueditor/ico...

The files dropdown looks good to me!

Okay I take some of that back. I See what its doing now -- the BUEditor is relying on sites/all/libraries/bueditor for the image source, but has cached them in the files folder.

What is interesting is why user 1 still doesn't see the icons on form when other users can. But I agree that we probably should have the BUeditor library added to the repo.

Nice sleuthing! Also, weird. No idea why BUEditor is not in sites/all/libraries... it's not there on prod, either.

Anyway, we shall chalk BUEditor weirdness up to "not our problem." :) The files drop-down is looking good to me on form-drupal, too. I think this is pretty close, but still needs review from drumm/dww.

I'm not sure what to review. The last attached patch is named foo.patch and comes with a remaining work list. And the issue summary needs deployment instructions if there is any work beyond applying a patch.

Here's the "mega diff" containing everything. Probably some of the bluecheese changes could be split off.

Ok, drumm checked this out and said:

1) He's cool with the approach taken in drupalorg_project_node_view() (though we should lose the "magic" comment. :P)
2) We should move the fieldgroup styling stuff upstream to project_issue.form_group.inc or whatever it is. And include the command drush ctools-export-revert field_group in the deploy instructions in order to make them 'stick.'

Markcarver also said to facilitate styling of the issue metadata fields, the fieldgroups should be laid out thus:

- Title
- Project
- [Field group w/ "issue-settings" class]
   - Category
   - Priority
   - Status
- [Field group w/ "issue-settings" class]
   - Version
   - Component
   - Assigned
- Tags

Reviewing this, is it possible to atleast merge the two lines of text in the issue metadata. I can understand if we don't want to merge the documents now as proposed. But lets atleast not have a text line in between forms it makes it soo much harder to spot and use. I am sure we can come up with a sentence that captures both.

"Follow Issue guidelines on status, priority and tags. Separate tags with a comma.

I don't think its necessary to be all finger pointy, on people using tags. The people that do this wrong, won't suddenly go "let me add some random tags", ohh the help text says I shouldn't "darn it, now I wont".

> The people that do this wrong, won't suddenly go "let me add some random tags",

I wish I knew WHAT the people who put random tags in are thinking!

@joachim I think they aren't, and that's not really something we can fix with text :)

It's another place where it would be useful if Form API had a built-in confirmation step as well as validation.

If the user has entered tags which don't exist yet, reload the form and ask them, 'You've entered these tags. Are you sure that's a good idea?'.

Why "punish" people that make legit use of the tagging system by adding an extra step for them? Not a good idea.

Just a quick bit of user feedback here:

1. It looked great in the OP where it was shown full width, but with the fixed width sidebar, I'm showing incomplete select lists / text fields (e.g. Issue status: Norma). Simple solution: nuke the sidebar. ; )
2. I know fieldsets aren't equivalent to vertical tabs, but it would be very helpful to be able to see some of the issue metadata without opening that fieldset, specifically the title and status. Is there anything we can do about that here?
3. The "Files" fieldset seems mislabeled, as it doesn't actually show / let you edit the files attached to the issue. It appears to be a simple file upload form, so why not keep the current name of the action link, "Upload new files" or something equivalent if we need to stick with the "nouns as fieldset labels" thing?

I can't wait for this to go in, though, as I just had to cut this comment from the thread and scroll back up to find the upload button to post it with the image. : P

...

5. The "Assigned" drop-down still feels like it better belongs grouped with the rest of the metadata related to the issue (title/category/priority/status). The other group should be project-related (project name/version/component).

6. Would it be possible to have it so after a certain width the project/version/component group wraps below the title/category/priority/status group?

I can't wait to see this go live too! All these things we mention can be minor followup tasks.

Btw, I stated this in the meta issue but in case people aren't following that, the week I had to throw at implementing this is gone. While it's possible I might have a fit of insomnia sometime that lets me do more work on this at some point, it's probably best for someone else to take over from here.

I'll take on this...Once I get my SSH key situation handled by Neil I'll be able to work on.

I have to unassign this from myself because I'm hitting a wall with step 3 > 7 "Use drush sql-sync @drupalorg.dev @drupalorg.local or drush pipe @drupalorg.dev @drupalorg.local (https://drupal.org/project/drush_sql_sync_pipe) to sync the database to your local machine" of https://drupal.org/node/2159409

I keep getting a key failure of some sort when executing this command and entering my password 3 times on my localhost git bash. Even though I could enter the devwww server with the same key no problem. It makes 0 sense so I'm going to have to focus my time on that before I can even consider testing and committing this patch.

Aaaaand another one bites the dust. Sorry, rayvan. Thanks for trying, anyway!

So we've got a working patch, deployed on a dev environment, but seemingly no more volunteers/volunteer hours to throw at this beyond the 100+ hours that have already gone in between creation of the mockup, reviews of said mockup by several high-profile contributors, coordination of volunteers to execute said mockup, testing, bug fixing, and troubleshooting environment errors.

How and when can the DA support getting this patch through to deployment?

I've asked eliza411. Drumm is occupied with critical Search API issues and thats basically the one resource we have. I hope priorities or budgets can shift, for the DA to step in at this point. Because I think its clear, even with our countless hours the hurdles around performing the last push are to big for a volunteer work force.

I don't think it's too big. The changes drumm asked for in #48 are actually pretty simple. But we need someone to step up to do it, and they need to be able to actually do the work. :)

Patch in 47 doesn't apply:

~/Sites/drupal_org joachim$ git co master
Switched to branch 'master'
~/Sites/drupal_org joachim$ git pull
Already up-to-date.
~/Sites/drupal_org joachim$ git st
# On branch master
nothing to commit (working directory clean)
~/Sites/drupal_org joachim$ pa diffydiff.diff
patching file sites/all/modules/drupalorg/drupalorg_project/drupalorg_project.module
Reversed (or previously applied) patch detected!  Assume -R? [n] n
Apply anyway? [n] n
Skipping patch.
3 out of 3 hunks ignored -- saving rejects to file sites/all/modules/drupalorg/drupalorg_project/drupalorg_project.module.rej
The next patch would create the file sites/all/modules/project_issue/js/project-issue.js,
which already exists!  Assume -R? [n] n
Apply anyway? [n] n
Skipping patch.
1 out of 1 hunk ignored -- saving rejects to file sites/all/modules/project_issue/js/project-issue.js.rej
patching file sites/all/modules/project_issue/project_issue.field_group.inc
Reversed (or previously applied) patch detected!  Assume -R? [n]

I'm rather confused...

More generally, working on d.org stuff at the moment is just too hurdly :(

Oh wait, is diffydiffdiff the diff against the current d.org code, and the repo I have from git is what webchick has been working on?

Argh, breakfast-time coding :/ Sorry for the noise!

I've done some work deploying sub-issues of this. The next thing to do here is re-roll from a fresh dev site.

Awesome, thanks a lot, drumm!

One disadvantage of doing that independently however is now the fieldset of issue metadata is collapsed by default on the node/X/edit form (which is unfortunately the primary purpose of using that form, and a fieldset which contains required fields), since the property to mark it un-collapsed by default doesn't exist without this backend code. :(

Filed a spin-off issue about that here: #2185739: [UX Regression] Issue metadata collapased by default on issue edit form, despite being the most common thing to change + containing required fields There were several contributors in IRC baffled by this all day today, so this might be a duplicate report.

I've been working on this for the week. It it now running on https://staging.devdrupal.org/. I'm doing some final testing and deployment soon.

I'll update this issue with a summary of the changes and commits next.

In project_issue:

• http://drupalcode.org/project/project_issue.git/commit/3cdada2 matched Drupal.org's field group configuration with project_issue.field_group.inc, so that the configuration could be reverted.
• http://drupalcode.org/project/project_issue.git/commit/64fca51 is another JS followup. Remembering the fieldset open/closed state doesn't make sense across issue commenting and editing. A prefix is added so those are stored separately. Usually, more fieldsets being open on issue creation is good.
• http://drupalcode.org/project/project_issue.git/commit/600feee updates field group names and file fieldset settings.
• http://drupalcode.org/project/project_issue.git/commit/c8d449b adds an issue edit block, which will be positioned in Drupal.org's content bottom region. This is based on the code here, but I went with a block so the form could have the full page width (hacking to break out of the column for this became too hacky). My comment from #31 did indeed work, we aren't turning off comments for issues, we are putting the comment form on a separate page which isn't visited.
• http://drupalcode.org/project/project_issue.git/commit/5747e95 updates the jump to link.
• http://drupalcode.org/project/project_issue.git/commit/f45c6b7 and http://drupalcode.org/project/project_issue.git/commit/19f8092 are followup commits.

In drupalorg:

• http://drupalcode.org/project/drupalorg.git/commit/8e7841f removes the #size updating, CSS forces the field widths.
• http://drupalcode.org/project/drupalorg.git/commit/1fc8f32 removes the "Update this issue" link.

In bluecheese:

• http://drupalcode.org/project/bluecheese.git/commit/d7af48f makes sure the content bottom region clears both the sidebar and the main content area. We've had this region for awhile, but I think this is the first use of it.
• http://drupalcode.org/project/bluecheese.git/commit/5ba09ac removes the hacks for the "Update this issue" link.
• http://drupalcode.org/project/bluecheese.git/commit/53c2488 arranges the form elements.

Deployed! Hope everyone likes it.

I think followups on the meta issue, #2159409: [Meta] Implement first iteration to improve issue queue workflow would be best to keep everything in one place.

Awesome work, everyone. Thank you! :)

Thank you @drumm!! This is AMAZING to finally have back on d.o!!

(This is Cottser editing Mark Carver's comment. The above comment was mine :))

Um... This is Mark Carver and I just posted ^^^ that message.

After this went live there have been a couple people including me seeing comments being misattributed.

https://drupal.org/comment/8461185#comment-8461185 was supposed to be barraponto
https://drupal.org/comment/8461187#comment-8461187 was supposed to be me, tim.plunkett

Also on that last issue, you can see that it no longer collapses when old files are hidden.

YESSSSSSSS!!!! :D :D :D :D

THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU THANK YOU!!!

(in case it gets mis-attributed, this is webchick. :))

Uh. Yes, yes it did. That's no good! :) - webchick

Ok this bug is now fixed. :) http://drupalcode.org/project/project_issue.git/blobdiff/19f80929aca6305...

#2191587: Fix mis-attribution errors following deployment of issue comment UX was filed as a sub-issue to clean up the mis-attributed comments that got posted during this ~2 hour window.

The "Add new comment" form for this node view is added underneath (in addition to) the regular node edit form.

I remembered we have other blocks like this configured to not show on node/*/*. I switched the block visibility to that instead of showing on node/*. This should also fix #2191589: Issue comment block showing up on revisions page.

Maybe this one caused #2191619: New on-page comment form is deleting all hidden files :( ?

Yep. :\

Btw, some process suggestions for next time. No judgment here, just trying to figure out how not to repeat mistakes:

1) No deployments on Friday nights. Go have fun instead. :)
2) No deployments of new things that haven't been on staging (and announced to the folks following a given issue) for probably at least 24 hours in order to let people bang on it a bit first.
3) We really do need to get those BDD tests back in action, because chances are good they could've caught both the comment mis-attribution and file deletion regressions prior to them happening on prod. (If not, they definitely should be expanded since it'd be great to never have those bugs again! :))

Wohoo! Congrats:)

What is the issue to get the BDD tests back?

Sorry, I'm not sure on that one.

Adding some of the related issues that have post-deployment regressions.

...or so I thought. :( Guess there's another one to add to the list. #2191833: Can no longer add "Related issues" by nid

Current regression list, for posterity:

#2191619: New on-page comment form is deleting all hidden files :( ("ouch!" critical)
#2191833: Can no longer add "Related issues" by nid (major/critical)
#2191825: Testbot results getting posted as unpublished comments by 'Anonymous' (major/normal)
#2191853: "View changes" link now a 403 (major/normal)
#2191587: Fix mis-attribution errors following deployment of issue comment UX (normal)
#2191589: Issue comment block showing up on revisions page (minor)

I've added this to the issue summary as well.

sun noticed #2192121: Previewing comments not possible

I did some cleanup of the new related/child issues to get them all under #2159409: [Meta] Implement first iteration to improve issue queue workflow.

1) No deployments on Friday nights. Go have fun instead. :)

Agreed, I should have followed this rule.

2) No deployments of new things that haven't been on staging (and announced to the folks following a given issue) for probably at least 24 hours in order to let people bang on it a bit first.

We use staging as "checked into our main codebase and ready to deploy." We haven't been using branches and master is always meant to be deployable.

When our staging/dev situation improves, this is the purpose of the "integration" or "beta" site that the SWG has proposed. In the meantime, keeping it available on dev is best. (And making it easier to log into dev.)

3) We really do need to get those BDD tests back in action, because chances are good they could've caught both the comment mis-attribution and file deletion regressions prior to them happening on prod. (If not, they definitely should be expanded since it'd be great to never have those bugs again! :))

Yep. I doubt everything would have been caught by BDD. Some of the regressions were quite unexpected, and I don't know that anyone would have thought to test for them.

Overall, I felt a lot of pressure to get this deployed ASAP! from a bunch of people and wanted to get it done.

I have some feedback on #88, but we're off-topic (sorry, my fault) so maybe we can either open another issue to discuss optimal process or just hash it out in IRC / DSWG meetings sometime.

Also, the final issue to complete the mockup as designed is #2192449: Improve edit Files UX.

We have another issue: #2146967: [Meta] Drupal.org Development Environments.

I don't think we should put effort into supporting localhost development without VMs; that will always have some amount of futzing with setup that could be spent developing. In the meantime, we can work with what we have, and work toward VMs.

I checked the test results that run after every deployment to git7staging, behat --tags=ci and there were actually no regressions from anything this weekend.

I totally like the good old collapsible fieldsets. Does not feel cluttered, but still all optons are there. Great!