Figure out and port project_issue comment numbering functionality to D7

Wait. At the moment, is this functionality NOT provided by Comment Easy Reply module?

If it's not, maybe it should.

Agreed. Comment_easy_reply is almost EXACTLY what I was going to propose for "linkable" comment numbers. I love it.

added some proposed solutions with pros/cons

Easy there, smarty pants. Project_issue is probably still going to need to do a touch of custom work via fields just for data migration purposes, but outsourcing to this new method adds the relationability [yes, new word] of linking a comment about a comment to that original comment. It's something I've always though was lacking in an issue comment, namely that when I say "Up in #32, you said..." but the "#32" part *doesn't link to anything*. This way, it would.

I'll get someone to do a security review first. Then, it's a deep dive to see exactly how this module handles it's data architecture, storage, and retrieval. No sense replacing the innards of project_issue's numbering system with something that doesn't improve the odds of adhering to TNG standards. Then, after that, we can open a 'pretty puhweeze' Infrastructure issue.

I'll get someone to do a security review first. Then, it's a deep dive to see exactly how this module handles it's data architecture, storage, and retrieval.

Please, no. Let's reverse those. If it doesn't match the needs well then let's not waste the security team's time.

Reviewing.

Comment easy reply does not have persistent numbering, so it would not work. The #s next to the comments depend on whether or not you can see unpublished comments, comments have been deleted, etc.

It's not usable in this state for our purposes.

Something sorta like https://drupal.org/project/serial but with the ability to put it on any entity (comment) and partition it by an entity property (comment nid) would work...

I've emailed the maintainer and asked to begin a discussion about modifying this module to incorporate a 1-to-1 comment ID to realnumber scheme for drupal.org. We shall see if they reply soon, and if not, we'll just make something happen during the upcoming sprint.

Hi everyone, I'm the maintainer of Comment Easy Reply module.
I'm really surprised (and honoured) to see you evaluating my module and I would really happy to talk with you about any possible improvement on the module.

1-to-1 persistent comment ID is a good idea, in my opinion it could be done in two ways:

1. Adding a new table with 3 fields: cid, nid and comment_number
2. Adding a new field comment_number to comment table

The second option sounds better to me because avoids creating a new table with as many rows as comment table has. Moreover, with a hook_shema_alter() on comment table schema, it should be possible to automatically load the comment number during comment entity load.

I'm really interested to hear your opinions and thoughts about this and all other new implementations/ideas you have on the module.

Hi Derek,
Thank to you for such interesting argumentations, there is always something new to learn :-)
I watched the problem mainly from a performance side, but now I see your concerns about schema changes by core modules.
I'm looking at project_issue_comments and its way of locking the table, it's very interesting and it works really well, but do you think it's a bad idea using the drupal standard locking mechanism (lock_acquire, lock_release, ...)?
What I'm thinking about is something like (I write meta-code to make it short)

// Comment has just been added.
if (lock_acquire('comment_easy_reply' . $comment->nid)) {
    // $next_id = SELECT FROM {comment_easy_reply_serial} WHERE nid=$comment->nid
    // INSERT {comment_easy_reply_number} VALUES($comment->nid, $comment->cid, $next_id)
    // $next_id++
    // UPDATE {comment_easy_reply_serial} set number=$next_id 
    // ...
    lock_release('comment_easy_reply' . $comment->nid);
}

I don't want to force anyone's decisions in any way, I'm just curious by nature and I love to hear from people who knows drupal a lot better than me.

It's ok for me to move the issue on Comment Easy Reply issue queue

Project_issue's locking code was added in 2009 while the 6.x lock.inc was added in 2010.

I think it makes sense to move to the core locking functions, but I'll leave it to dww/hunmonk to say if they feel #14 makes sense or something more like what project_issue has now makes sense. I did just check and there are no messages in drupal.org's dblog currently for project_issue meaning that the locking code has not failed to grab a lock, though our site only holds less than 24 hours of data so that's hardly a conclusive test.

The discussion and implementation details have moved to #1678398: Persistent (atomic) numbering of comments needed

I'm not convinced. We have special link handling already for comments, that work the same way as project links.

Here is an implementation of using {comment}.thread as the basis for numbering.

I have already fixed the data glitches on drupal.org that would prevent rethreading issues to match the comment_number. (assuming more don't crop up.)

Furthermore, supporting threading is a matter of adjusting the regexp slightly.

Hi bdragon,
the idea of using comment thread to get comment number is brilliant!
I just finished to implement {comment}.thread numbering in Comment Easy Reply.
At this stage I don't know if it could still be interesting for you, anyways it seems to work pretty well. With the new comment numbering system I've been also able to give a proprer link fragment in comment's permalink.

Here's an updated patch that should play nice with comment_easy_reply.

Also removed a todo that is todone.

@chx, could you please review #20 before we commit it?

Derek, I am not sure whether it is such a problem that we reuse the comment numbers after deletion. I have found a bit weird we ddn't, as the issue you linked has a huge gap in the numbering because of spammer cleanup.

dww: Don't worry about that, part of the migration will be to re-thread the issues to match comment_number. I already have it under consideration. I spent several hours a while back combing through the database and cleaning up the glitches that would prevent rethreading like this.

dww: And for reassigning a comment number, who cares? The only possible place this could matter is a spam delete and a followup referencing the spam comment happening simultaneously causing the followup to reference itself... Big deal...

Back to me.

I tweaked the migration job that strips the extra comment titles, hopefully it won't bail now.

I committed a modified version of #20 which is closer to how core does the permalink, because I can't remember why I was attempting to preserve the query parameters. If some reason does come up it can always be changed again.

http://drupalcode.org/project/project_issue.git/commit/f3665dc65b6f55f1a...

Bluecheese will need to be updated...

@bdragon, could you please elaborate on how the Bluecheese Team needs to update their stuff based upon what you've done in commit #31?

@Senpai: Here is one possible implementation.

To elaborate on that patch:

If we want to avoid having a separate tpl for issue comments, we can alter the link into the comment title, to make up for the fact that bluecheese discards the permalink.

I've cross-linked the Bluecheese Team (posted in #1796078: Re-theme project and issue pages to the patch in #33 above, so I'm marking this 'fixed' because there seems to be no further development needed.