Project page:
https://www.drupal.org/sandbox/chrisolof/2385845

Git clone command:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/chrisolof/2385845.git tracking_number

This module provides a field for storage and display of shipment tracking numbers. It is intended for Drupal 7.x.

I believe there is no other module out there that handles field storage and display of shipment tracking numbers.

What about simple_package_tracking you ask? simple_package_tracking is not a field type module, nor is it aiming to be. Instead, it's simply a commerce and ubercart addon module to allow storage of shipping tracking number data on orders, and orders only.

This module, Tracking number, aims to be a field type and nothing more. And as a field type, it can be attached to any fieldable entity - not just orders. It's also integrated with the Entity API, which means it has out-of-the-box support for deep token access, rules field data comparison and manipulation, as well as a multitude of other contrib modules that make use of the Entity API to access field data, making this field-approach to storage of tracking number data potentially more desireable than simple_package_tracking for some Drupal projects.

This is my first project application, but I'm a long-time Drupal contributor in the issue queues.

Comments

PA robot’s picture

PA robot commented
Status: Needs review » Needs work

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxchrisolof2385845git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

chrisolof’s picture

chrisolof
he/him
Primary language English
commented
Status: Needs work » Needs review

Ok - all valid issues outlined by the automated review tools have been addressed. Ready for review.

artsakenos’s picture

artsakenos commented

Hello Chrisolof and thank you for yur contribution, please check the following:

Automated Review
Pareview is still giving some error and warning on the .migrate file, please check:
http://pareview.sh/pareview/httpgitdrupalorgsandboxchrisolof2385845git

Individual user account
OK! You can read https://www.drupal.org/node/272587

No duplication
Please explain better why there are no similarities with your module and simple_package_tracking (seems like it allows to add this information to the order page) or uc_tracking. Also you can add them to the project description as a reference.

Master Branch
OK! you can check: https://www.drupal.org/node/1127732

Licensing
OK!, you can check: https://www.drupal.org/licensing/faq

3rd party code
OK! See: https://www.drupal.org/node/422996

README.txt/README.md
I also think it’s OK! See: https://www.drupal.org/node/2181737

Code long/complex enough for review
OK! https://groups.drupal.org/node/195848

Secure Code
I see no security issues. OK! https://www.drupal.org/writing-secure-code.

Style of Code
There is no admin panel to update the tracking links. I think it would be better, even if it is also a good idea to release updates in order to synchronize the module.

Project Description
Please write your machine name for making the checkout easier, and quickly recognize the module in the folder.
e.g., git clone --branch 7.x-1.x http://git.drupal.org/sandbox/chrisolof/2385845.git tracking_number

jribeiro’s picture

jribeiro commented

Hi chrisolof,

Like #3 said, you have some issues related with automated Review.

Manual Review: 

/**
 * Defines available shipping providers/organizations.
 */
function tracking_number_organizations() {
  return array(
    'usps' => t('United States Postal Service'),
    'ups' => t('UPS'),
    'fedex' => t('FedEx'),
    'dhl-us' => t('DHL'),
    'dhl-global' => t('DHL Global'),
  );
}

Why not turn this function to a hook info? Só on that way, other modules can define any other organizations. Or, another approach is to set a admin area to define these organizations.

################################ Coder Sniffer #################################

FILE: /home/jribeiro/Projects/git/2385845/tracking_number.migrate.inc
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 11 | WARNING | Line exceeds 80 characters; contains 83 characters
--------------------------------------------------------------------------------

############################### DrupalPractice #################################

FILE: /home/jribeiro/Projects/git/2385845/tracking_number.migrate.inc
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
 37 | WARNING | Class name must be prefixed with the project name
    |         | "TrackingNumber"
--------------------------------------------------------------------------------
chrisolof’s picture

chrisolof
he/him
Primary language English
commented
Issue summary: View changes
chrisolof’s picture

chrisolof
he/him
Primary language English
commented
Issue summary: View changes

Added difference between this module and the pre-existing simple_package_tracking contrib module. Clarified project goals.

chrisolof’s picture

chrisolof
he/him
Primary language English
commented
Status: Needs review » Needs work

@artsakenos and @jribeiro - Thank you for reviewing this! I believe all of the issues you've identified have been addressed except for the ability to add new shipping organizations. I really like the hook idea for defining additional shipping organizations, so stay tuned for the addition of a tracking_number.api.php file exposing that functionality.

Changing status to Needs work until that's done.

chrisolof’s picture

chrisolof
he/him
Primary language English
commented
Status: Needs work » Needs review

Ok - I believe all issues have been addressed now and this is ready for review once again. Details:

@jribeiro - I've added a hook to allow for adding to or manipulating the shipping organizations (including their package tracking urls) and tested it to verify it works. See hook_tracking_number_organizations_alter() in tracking_number.api.php.

I've also fixed the documentation length issue and class namespacing issue in tracking_number.migrate.inc. What's odd is that in every example I've seen of adding a migrate field handler to support a custom field type the class name is prefaced with "Migrate" instead of the module's name. But I think that convention, as DrupalPractice points out, is not technically correct and the field handler works just as well with the correct namespacing :)

@artsakenos
Automated Review
http://pareview.sh seems to not like the code samples provided in the migrate integration documentation - possibly because they're beyond one-liners. After more research I believe they are OK per https://www.drupal.org/coding-standards/docs#code

No duplication
See new issue description for an explanation on why I believe this does not duplicate simple_package_tracking or uc_tracking. Briefly: this module is simply aimed at exposing a new field type, while simple_package_tracking and uc_tracking are feature-addons to commerce frameworks.

Style of Code
The ability to update tracking links has been provided via hook_tracking_number_organizations_alter() in tracking_number.api.php.

Project Description
The default destination folder has been added to the git clone command example in the issue description.

Also to note: Integration with the Entity API has been added, which gives a multitude of contrib modules access to and support of this new field type (eg. Token, Rules, Views, Search API, etc.).

jim.applebee’s picture

jim.applebee commented

Automated Review

2 Best practice issues identified by pareview.sh / drupalcs / coder. Results can be ignored

Manual Review

Coding style & Drupal API usage
List of identified issues in no particular order.
  1. (*) Module has a dependency to the field module, but not specified in the info file
  2. Add ability to choose the allowed shipping providers/organization as configuration.
jim.applebee’s picture

jim.applebee commented
Status: Needs review » Needs work

Changing Status

jim.applebee’s picture

jim.applebee commented

I would have prefered if the module had a configuration page that allowed the users to choose the allowed shipping providers/organizations, so that users can use the module out of the box instead of writing a hook to just to disable some of the items from the list.

jim.applebee’s picture

jim.applebee commented

I think it would be better if your field configuration had options to choose the allowed values for shipping providers, instead of my previous suggestion of a separate configuration page.

PA robot’s picture

PA robot commented
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.

chrisolof’s picture

chrisolof
he/him
Primary language English
commented
Status: Closed (won't fix) » Needs review

@Drupalbee Field module dependency added - good catch!

I feel like the ability to add or remove organizations can already be achieved through hook_tracking_number_organizations_alter(). Although the ability to disable certain shipping providers in the GUI on a per-field basis would be a nice feature, it certainly feels like more of a non-essential feature request that could be achieved in a future release of the module if there's a real desire for it.

ayesh’s picture

ayesh commented
Status: Needs review » Reviewed & tested by the community

Hi Chris,
Installed, tested, and the module worked great. Thanks for the contributions.

The module is well written, certainly one of the best ones I've seen for some time. Thanks for the hard work!

Here are few suggestions, but none of them are blockers.
- In the field schema, the tracking number allows NULL values, but looking at the hook_field_is_empty implementation does require to fill the number field.
- Since the shipping organization is now a select list, I cannot imagine a situation that the tracking_number_field_validate's tracking_number_org_invalidwould be fired. Drupal core select lists has a validation to make sure that the selected value was present in the #options array.

Everything else looks great to me. Since there are other reviews already, I'd go ahead ahead RTBC this.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Reviewed & tested by the community » Fixed

Thanks for your contribution, Chris!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.