Group together files posted in the same revision.

I tend to agree ... however, the comment ID's (and associated jumplinks) are added to the table by project_issue, and are not native to the file_metadata_table module itself; so any logic used to group the rows together based on comment_id probably belongs there as well.

The file_metadata_table module has an alter() hook which can be used to adjust the structure of the $rows array; so we could add a project_issue_file_metadata_table_output_alter() call which groups the rows by comment id as needed. Therefore, I'm going to move this over to the project_issue queue.

However, another item to consider is that we currently haven't yet implemented a 'show/hide files' capability when editing an issue. Once this is in place, the intended workflow would be to hide patches from view as they are rendered obsolete by subsequent patches in an issue thread. As a result, if users abide by this workflow, the table of files should never approach the length that is illustrated on the dev site ... the file_metadata_table formatter already supports hiding the non-relevant files in a secondary table located within a collapsed fieldset; which will significantly improve the experience relative to the dev site example.

I don't think we need this for launch. I'm not even totally sure it's a good idea. For example, we want separate rows in this table for each patch uploaded, even if they're in the same comment, so that the test bot can do its thing in peace. Maybe there's some other grouping beyond trying to smoosh everything into the same row. Maybe we can play tricks with special-casing interdiff, but if we're going to do that, I'd rather just implement functionality on the site that tries to automatically generate interdiffs (I don't remember seeing a feature request about that, but someone might find something via searching -- if so, feel free to x-link it here).

Anyway, we can leave this tagged for the d.o D7 port, but I'm downgrading to minor make it clear this isn't a blocker. #1833684: Need a file widget that gracefully handles lots of hidden files (which resolves #1245508: Issue summaries: Allow outdated attached files to be hidden/replaced/flagged.) is much more urgent.

Thanks,
-Derek

@klonos: I really appreciate your interest in all of these problems. However, I'm going to firmly request that you carefully read the current plan before writing more comments like this. ;) I don't want to keep explaining why your concerns are either misplaced or already addressed with the approach we're in the middle of implementing.

Thanks,
-Derek

p.s. That was slightly harsh, sorry. "why your concerns are either misplaced or already addressed" should say: "why most of your concerns..."

Anyway, the basic point is still true. :) For now, I'm going to spend my time implementing the current plan and then you can tell us if/why it's not sufficient once you actually see it working.

Thanks/sorry,
-Derek

To respond to #4, the file_metadata_table keys off of the file 'status' flag to determine whether the file should be displayed or not. Thus, while editing an issue, a user would flag any existing files which are obsoleted by the patch currently being uploaded (via a custom widget, still to be developed).

Thus, the display doesn't truncate based solely on number of files ... it would be up to users to prune the display.

Yes, moving to a better queue.

Related:
#1962930: Make the file upload widget as flexible as the display formatter: alter hook, configurable table columns
#1961712: Notices and bugs when files are deleted without a new node revision.

I'm tempted to remove the d.o D7 tag from this, but so long as we leave this minor/feature request I think we're okay.

p.s. I don't see what screenshot #10 has to do with this feature request. Sounds like you're talking about either #1956166: Auto-generate interdiffs between patch files (from patches uploaded previously). or the hidden files stuff that's already done and working. I'm not seeing how that helps with grouping files that were uploaded with the same node revision which is what this issue is about.

Since the user and autogenerated comment # will be the same, perhaps we really want a row for each node revision where we list all the shared info once (including perhaps a timestamp), with a nested row for each file attached in that revision. A mockup of that would be useful. We haven't done any mockup revisions in a long time, and I think a final pass with the UX team before launch would be important.

Thanks,
-Derek

While I initially punted this to project_issue (based on us not having the cid), I can support this as a setting which allows grouping of files which were added in the same revision.

However, any implementation for drupal.org is going to have to consider how PIFT integrates to provide patch testing status ... so I'd like to wait until after that is implemented before tackling this, so as to not paint ourselves into a difficult corner.

Some mockups to help visualize the final product with full PIFT integration might help bring me around, so I won't straight-up postpone this. :)

Yes there's an operations column.

As I've said, the mockups aren't final, haven't been looked at in a long time, and need to be updated again soon as we're getting close to something that's viable for launch.

This has been implemented within pift_extended_file_field_output_alter(), which is an alter hook exposed by the Extended File Field module. See #1979574: Testbot info display on issues for the resulting output.

This should give us what we need on drupal.org, so I'm marking it as "won't fix" for now; in lieu of a more effective solution being implemented elsewhere.

For the reasons outlined in comment #1, I don't think I want to pursue this further within the extended_file_field module itself, as the 'comment id' is not native to this module, and the grouping of files added in the same revision feels like a very minor edge case ...

That said, if you feel strongly that the grouping of files from the same revision into the same row would still be a valuable feature within the Extended File Field module itself, feel free to re-open it.

Another thing to consider is that extended_file_field allows you to hide files from that list ... so for the intended drupal.org workflow, only the most current/relevant patches should appear in the file table; and ideally, we shouldn't have many long file listings.

This will have to wait until after launch unless someone volunteers to work on it.

I'm wondering if the current operation may be good enough, with the zebra striping identifying which files were added in which revision. klonos, can you look at git7site.devdrupal.org and comment on whether the distinction is enough (so that we can decide whether to close this issue)?

Untagging, let Dev Tools review this and decide if it's needed or not.

This looks doable within pift_extended_file_field_output_alter(), which is where the striping is implemented. Instead of hard-coding the rowspan to 2, it will have to be variable.

Since there is the striping, moving down to minor priority.