git clone --branch master git.drupal.org:sandbox/bloke_zero/1504400.git pubdate
http://drupal.org/sandbox/bloke_zero/1504400
Initially Drupal 6.x, D7 port in progress.

What is Pubdate?

Pubdate was developed as a solution to a common problem facing publishers, how to publish the online version of a print magazine using a simple and robust admin interface.

The module for replicates a magazine type relationship between content allowing the nomination of parent and child nodes, for example front_cover and in_this_issue, where the content is published together depending on the dates specified for the parent node. The use case is for a magazine publisher wanting to mirror their publishing cycle on line, so content is gathered together in issues, and often presented as a cover and articles.

Features

  • Content can be loosely organised into 'issues' and can be displayed in a view as a complete issue with the parent and child nodes, for example, with a views attachment display
  • Simple admin interface for nominating parent and child content types (more than one content type can be nominated as a child)
  • The Date module is a dependency so the date fields are standard Drupal date fields and so can use the date picker etc.
  • Providing parent/child relationship means that dates can be entered only once for the parent and inherited by children
  • Child nodes can override the parent dates in 2 ways, either individually or for nodes that need to stay published after the parent has been unpublished can be defined by type
  • Pubdate fields are exposed to views for display, filtering and sorting
  • Admin views where nodes are grouped by 'issue' can be created
  • CCK node_reference can be removed and the parent/child relationship provided by pubdate can be used via views arguments

Other modules

Scheduler

Scheduler is a more general module and has some deficiencies when used in a publishing content, for example scheduler deletes its own data when the scheduling is over, this module keeps that information and exposes it for use, for example, in an admin view. It also doesn't provide the relationship between content.

Rules etc.

This functionality could be created using Actions, Rules and workflow but this module provides a simple one stop solution to a common use case for publishers

Auto expire, node expire

This module is different in scope and intention, these modules expire content, pubdate also organises content.

Project reviews

http://drupal.org/node/1505236#comment-5880984
http://drupal.org/node/1517856#comment-5915284
http://drupal.org/node/1529386#comment-5885590

Comments

ourwayoflife’s picture

ourwayoflife commented

You are working on a master branch, so you must move your code to a major version branch. You can follow this how-to: http://drupal.org/node/1127732

./pubdate.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming

./pubdate.admin.inc: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming

Remove all old CVS $Id tags, they are not needed anymore.

There are other stuffs to fix. Details here: http://ventral.org/pareview/httpgitdrupalorgsandboxblokezero1504400git

Manual review:
* Only you can clone your repository? It asks for password.

musikpirat’s picture

musikpirat commented

Seems you deleted all your content from git: The master branch contains a README.txt with instructions but nothing else.

bloke_zero’s picture

bloke_zero commented

Hi,

Try git checkout 6.x-1.x - that was how I read the instructions for setting up modules in gits - nothing in the master branch except the readme and a branch per major version release.

Thanks

Alex

bloke_zero’s picture

bloke_zero commented

Those are all fixed now:

git clone --branch 6.x-1.x bloke_zero@git.drupal.org:sandbox/bloke_zero/1504400.git pubdate

Except for class names inherited from views which I'm extending.

I don't think I'm working in the master branch, but I could be wrong!

Thanks

bloke_zero’s picture

bloke_zero commented

Ok, cleaned up git repository and resolved and Drupal Coding Standards problems except for where I'm extending views (see here for a discussion of that issue http://drupal.org/node/1366844 )

bloke_zero’s picture

bloke_zero commented
Status: Needs review » Needs work
bloke_zero’s picture

bloke_zero commented
Issue summary: View changes

corrected repo name

bloke_zero’s picture

bloke_zero commented
Status: Needs work » Needs review
Issue tags: +PAreview: review bonus

git clone --branch 6.x-1.x git.drupal.org:sandbox/bloke_zero/1504400.git pubdate

Thanks for looking!

patrickd’s picture

patrickd commented
Issue tags: -PAreview: review bonus

https://drupal.org/node/1537696#comment-5885318

I can't accept this as review as the reqirement for the review bonus is a manual review, meaning not just pointing to an automated report.

Removing tag, Sorry.

patrickd’s picture

patrickd commented
Issue summary: View changes

Adding project reviews for bounus

bloke_zero’s picture

bloke_zero commented
Issue tags: +PAreview: review bonus

@patrickd - No problem, substituted http://drupal.org/node/1517856#comment-5915284 above. I can see the queue is crazy!

luxpaparazzi’s picture

luxpaparazzi commented
Assigned: Unassigned » luxpaparazzi

Automatic review:

Review of the 6.x-1.x branch:

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.


FILE: ...areview_temp/test_candidate/includes/pubdate_handler_filter_current.inc
--------------------------------------------------------------------------------
FOUND 5 ERROR(S) AFFECTING 4 LINE(S)
--------------------------------------------------------------------------------
  8 | ERROR | Class name must begin with a capital letter
  8 | ERROR | Class name must use UpperCamel naming without underscores
 12 | ERROR | Method name "pubdate_handler_filter_current::value_form" is not
    |       | in lowerCamel format, it must not contain underscores
 25 | ERROR | Method name "pubdate_handler_filter_current::admin_summary" is
    |       | not in lowerCamel format, it must not contain underscores
 30 | ERROR | Method name "pubdate_handler_filter_current::operator_form" is
    |       | not in lowerCamel format, it must not contain underscores
--------------------------------------------------------------------------------


FILE: ...w_temp/test_candidate/includes/pubdate_handler_parent_pubdate_title.inc
--------------------------------------------------------------------------------
FOUND 4 ERROR(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
 11 | ERROR | Class name must begin with a capital letter
 11 | ERROR | Class name must use UpperCamel naming without underscores
 15 | ERROR | Method name
    |       | "pubdate_handler_parent_pubdate_title::option_definition" is not
    |       | in lowerCamel format, it must not contain underscores
 24 | ERROR | Method name "pubdate_handler_parent_pubdate_title::options_form"
    |       | is not in lowerCamel format, it must not contain underscores
--------------------------------------------------------------------------------
luxpaparazzi’s picture

luxpaparazzi commented
Assigned: luxpaparazzi » Unassigned
Status: Needs review » Needs work
Issue tags: -PAreview: review bonus

1. It's uncommon adding comments as separators into .info files, I recommand removing those lines

2. It's better removign "--" from t(), and concatanating it after:
$pubdate = t('--NOT SET--');

3. Removing "*" as indicated (repeat on other places):

/**
 * Additional node form validation if node type is child
 *<del>*</del>/

4. "want as parent. As you" => remove double-space in phrase

5. At the beginning i was confused about the missing "break", but later i thought it would be correct. I recommand a one-line comment making this clear for another reader

      case 'insert':
        $insert = TRUE;
      case 'update':
        // Get pubdata.
        $pubdata = pubdate_get_pubdata_from_node($node);

6. Personal opinion: i would at some blank lines at specific places for making room for code-readers

7. In the Drupal-world people prefer using TRUE and FALSE instead of 1 and 0

       $status = '0';
    }
    else {
      $status = '1';

8. Doc-comments are not for splitting sections of code (also at other places)

/**
 * Private helper functions
 */

9. I can see the function name at the function declaration, don't need it in the doc-section, here you should give a small explanation about the function. (also on other places)

/**
 * pubdate_auto_get_nid
 *
 * returns the NID from the auto complete field
 *
 * @return int
 *   stripped nid
 */
function pubdate_auto_get_nid($string) {
  return trim(drupal_substr($string, strpos($string, '[nid:') + 5), ']');
}

10. "0" should probably be FALSE?

  if (!$pubdata['unpubdate']) {
    $pubdata['unpubdate'] = 0;
  }

11. also check the automatic review above!

---
The response time for a review is now approaching 4 weeks.
Get a review bonus and we will come back to your application sooner.
See: http://drupal.org/node/1410826

You could for example start by evaluating my own project:
http://drupal.org/node/1302786

bloke_zero’s picture

bloke_zero commented

@luxpaparazzi re. comment #10, as these are inherited from views (I'm extending classes defined in the Views module) I can't fix these. Well, I could, but I don't feel like it would make sense - see here for a discussion: http://drupal.org/node/1366844

Thanks

luxpaparazzi’s picture

luxpaparazzi commented

these are inherited from views (I'm extending classes defined in the Views module) I can't fix these. Well, I could, but I don't feel like it would make sense - see here for a discussion: http://drupal.org/node/1366844

@bloke_zero => ok, it's not your job for correcting views-module at this place

bloke_zero’s picture

bloke_zero commented
Status: Needs work » Needs review

@luxpaparazzi - thanks :-) - I appreciate the rest of the comments and have implemented them.
git clone --branch 6.x-1.x git.drupal.org:sandbox/bloke_zero/1504400.git pubdate

  • Added space for code legibility
  • Fixed a bug with overriden nodes
  • Improved comments throughout
  • Improved field descriptions in forms
  • Replaced 1/0 with TRUE/FALSE
klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work
Issue tags: +PAreview: security

Sorry for the delay. Make sure to review more project applications and get a new review bonus and I will get this finished immediately.

manual review:

  1. pubdate_uninstall(): why do you install a schema here?
  2. pubdate_install(): you should call drupal_install_schema() only once with your module name. Same for pubdate_uninstall().
  3. pubdate_parent_settings_form(): #options on checkboxes is not autmatically sanitized by the form API in Drupal 6. As node type names are user provided input you need to sanitize them to avoid XSS exploits. See http://drupal.org/node/28984

Setting this to "needs work" because of the security issue, but otherwise it looks nearly ready to me.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Issue summary: View changes

update reviews