Needs review
Project:
Re: Comment subjects
Version:
7.x-2.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
27 Jan 2010 at 00:38 UTC
Updated:
8 Sep 2011 at 13:43 UTC
this issue will be to follow up the 7.x-2.x port (for branch 2.x, of course)
which is the one with:
- token support
- per content type settings
Comments
Comment #1
arhak commentedAlmost there!
- token UI presentation (?)
- upgrade path
-
gross migration (bring some functionality)---
migrate.infofile (remember to declarefiles[])---
test migrated stripping algorithm---
migrate to DB TNG---
update to new FAPI behaviors---
migrate theme functions---
replace code conditionally reliying on token module to rely on core's tokens---
drop module_load_include to rely on core's "tokens" grounp (hook_hook_info)-
CVS---
create tagDRUPAL-6--2to separate 6.x-2.x branch while keeping HEAD for 7.x-2.x---
remove from CVS oldcomment_subject.token.inc(history is not relevant, total refactor)---
add to CVS newcomment_subject.tokens.inc(history is not relevant, total refactor)- token UI presentation (partially done)
---
algorithm to retrieve a human readable table of token and their chains----- pending: efficiently look for succeeding chains
--- fieldset are not collapsible within vertical tabs (workaround?)
--- currently table is presented totally expanded (testing purposes)
--- review other modules' approach
--- would be there a token_ui module for D7? #514990: Add a UI for browsing tokens
-
Token Support---
known bug: revisit replacement validator to make it work with token's chains as well---
extend token support:-----
known bug: for [comment:uid] and related tokens (e.g. [comment:name], [comment:user:*]) AKA "Notice: Undefined property: stdClass::$uid"-----
known bug: attempting to load an entity from an empty string id: comment_load($comment->pid) AKA "Warning: array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController->load"---
what used to be:-----
[comment-parent-title] won't match core's [comment:parent:title] behavior-------
provide own [comment:parent-title] to alternate between [comment:node:title] & [comment:parent:title] behavior accordingly-----
[comment-parent-auto-numbering] now would be [comment:parent:numbering]-------
BUT having no parent at all should return zero-----
[comment-pid] is now provided by core as [comment:pid]-------
BUT is empty instead of zero when there is no parent comment---
review integrity between supported tokens & tokens which requires disable comment subject-----
some tokens like [comment:mail] and other related to [comment:uid] differ:-------
if anonymous user may post comments: they will require comment subject disabled-------
what if permission to post comments is granted to anonymous afterwards?-------
ensure consistency (test how does 6.x version behaves regarding this)- Upgrade Path
---
upgrading from 6.x-1.x or 7.x-1.x will NOT require any update (default legacy behavior)---
upgrading from an earlier 6.x-2.0-alphaX version will require to upgrade to 6.x-2.0-alpha5 first (since 6.x-2.0-beta1)--- upgrading from 6.x-2.0-alpha5 and above only requires token migrations (variables will be named the same)
--- who is gonna be responsible of providing algorithm to migrate old tokens to new ones?
----- consider manual migration for non-core tokens
----- consider whether to use token_replace with 'clear'=>TRUE or not (maybe configurable? per content type?!)
--- this module as a token provider:
----- should expose a function to migrate its old tokens to new ones
----- any convention? how could other modules take advantage of it?
--- this module as a token consumer:
----- how to migrate tokens provided by who knows whom?
--- every token used will be non-sanitized and afterwards check_plain
----- document the impossibility to use previous non-raw versions of tokens
-
documentation---
review consistency between README.txt, hook_help and actual behavior- UI consistency
--- D7 renamed comment_subject_field option as "Allow comment title" but other places refer to "comment subject", should this module "alter" it to read "Editable comment subject" or something? or will D7 be release with more consistent UI strings post-alpha1? changeing it might provoke users lost when reading guided steps and won't be able to find the checkbox "Allow comment title", then..?
-
General---
missing update for disabled subject on hook_comment_update (requires backport to 6.x-2.x as hook_comment($edit, 'update'))---
helper functions are based on $comment since most of them will be invoked with a fake comment being created but non-existent yet, nevertheless there are cases where handling $cid would avoid DB hits and therefore both should be supported by helper functions (requires backport to 6.x-2.x)NO: reviewed and the statement doesn't holds-
backport to 6.x-2.x---
use $node->comment_count populated on comment_node_load instead of deprecated function comment_num_allNO: doesn't counts unpublished comments (another approach was taken)---
use $node->type populated on node_load instead of node_type_get_type($node)->typefollow up #571654: Revert node titles as fields
if it gets committed in D7 it will brake this module
(currently not committed and seems to be candidate for D8)
Note: this first comment will be edited continuously to reflect ToDo list updates
Comment #2
arhak commentedplease do NOT use direct "reply" link of comment #1 to keep it editable in time
feel free to add comments bellow
Comment #3
arhak commentedEDIT: moved to comment #1
Comment #4
arhak commentedwell, we now have an alpha release for testing
remaining issues:
- token UI presentation (???)
- upgrade path
- UI consistency (should be "title" for UI, but still is "subject" for code)
Comment #5
arhak commentedsorry, wrong title
Comment #6
drikc commentedHello, where is the branch 7.x-2.x? The git repository doesn't hold one with that name.
Comment #7
arhak commented@#6
for CVS it was HEAD,
for Git it is Master