Just checking. Would be nice to have this, if only as part of managing an upgrade to D7.

CommentFileSizeAuthor
#13 qa_checklist-1031710-13.patch53.98 KBtraviscarden
#12 QA_Checklist-Port-1031710-12.patch6.76 KBmgifford
#10 qachecklist-upgrade-d7.patch6.13 KBperfectweb
#6 qachecklist_module_upgrade_d7-1031710-6.patch5.97 KBmgifford

Comments

mehtatejas’s picture

mehtatejas commented

subscribing

midmood’s picture

midmood commented

subscribe

midmood’s picture

midmood commented

subscribe

eidolon night’s picture

eidolon night
Location Batavia, NY
commented

I've been busy lately, but I'll work on getting this up to D7 in the next couple months.

jakonore’s picture

jakonore commented

sub

mgifford’s picture

mgifford
he/him
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new qachecklist_module_upgrade_d7-1031710-6.patch5.97 KB

Ok.. Here's a patch. Seems to work fine for me.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Anyone going to test this & put out a release?

perfectweb’s picture

perfectweb commented

I download the latest 6.x branch today and applied the patch in #6. After installing the module I was prompted with an error telling me that this module wasn't meant for Drupal 7.x.

I noticed that the .info file had two lots of version=7.x and version=6.x lines that were marked as being added by the drupal packaging script. Remove the lines related to 6.x and refreshing the modules page allowed me to enable the module successfully.

Once enabled the module appeared to work correctly.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Can you roll a new patch with the correction?

perfectweb’s picture

perfectweb commented
StatusFileSize
new qachecklist-upgrade-d7.patch6.13 KB

Here is the patch that resolves the issue. This patch replaces the one in #6

traviscarden’s picture

traviscarden commented
Title: Drupal 7? » Port QA Checklist to Drupal 7
Issue tags: +D7 porting
mgifford’s picture

mgifford
he/him
Primary language English
commented
StatusFileSize
new QA_Checklist-Port-1031710-12.patch6.76 KB

Ok, so this didn't install against git, so I re-rolled it.

$ git apply qachecklist-upgrade-d7.patch
error: patch failed: qachecklist.info:1
error: qachecklist.info: patch does not apply

But there are issues with the .install file when running through Coder:

sites/all/modules/patched/qa_checklist/qachecklist.install
qachecklist.install

    severity: criticalclick to read moreLine 127: In SQL strings, Use db_query() placeholders in place of variables. This is a potential source of SQL injection attacks when the variable can come from user data. (Drupal Docs)

      db_query("INSERT INTO {qa_checklist} $task_fields VALUES (1, 1, 'Security Review', 'security_review', 'http://drupal.org/project/security_review', 'admin/build/modules', '', 1)");

Now not sure how much a fear there is of injection attacks from an .install file, but worth noting that it's not a best practice.

The other files seem fine.

traviscarden’s picture

traviscarden commented
StatusFileSize
new qa_checklist-1031710-13.patch53.98 KB

Great start, @mgifford. Here's another patch with the following changes:

  • Changed module machine name to match project name. (i.e. Changed qachecklist to qa_checklist.) A 6.x to 7.x port, when nobody's using the major branch yet, seems like the perfect time to fix a schizophrenic project. :)
  • Refactored qa_checklist_install() to define tasks in a (more readable, less redundant) array format, and replaced db_query() calls with drupal_write_record(), eliminating SQL injection vulnerability complaints from Coder.
  • Fixed numerous tasks with broken config paths or missing info.
  • Removed or updated tasks for modules that got moved into core (e.g. Admin Role and CSS Gzip).
  • Removed now extraneous page compression task.
  • Fixed help text apparently leftover from repurposing SEO checklist module.
  • Removed the files and code that are automatically added by the Drupal.org packaging system.
  • Made several other small code and interface improvements.
  • Fixed all coding standards issues per PAReview. (!)
mgifford’s picture

mgifford
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

I think it's missing the version number in the .info file. Think this should be version = "7.x-3.0-dev" but that's a pretty trivial thing.

This is good to go I think. Really to see many of the changes you brought in. It's a way more comprehensive patch than what I contributed.

I've tried to reach out to the developer.

traviscarden’s picture

traviscarden commented

Thanks, @mgifford!

The version property is actually added by the drupal.org packaging system. Thus we are discouraged from including it manually.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Ahh.. Good to know.

traviscarden’s picture

traviscarden commented
Status: Reviewed & tested by the community » Postponed

Getting under the hood for this patch motivated me write an abstraction for QA Checklist and SEO Checklist to share. Please weigh in on #1571642: Re-implement on Checklist API?.

traviscarden’s picture

traviscarden commented
Version: 6.x-6.x-dev » 7.x-1.0-rc1
Status: Postponed » Fixed

All right! @Eidolon Night has kindly granted me maintainer rights to the checklist. I've completed the port and uploaded a release candidate. Please try it out (you'll need the new Checklist API) and provide feedback in the issue queue. If there are no bug reports in a few weeks, I'll make it a full release. Thanks for everyone's help!

UPDATE: I've created an issue to completely rethink the contents of the checklist. Please weigh in at #1645936: Re-invent QA Checklist.

mgifford’s picture

mgifford
he/him
Primary language English
commented

Great to hear that there's a D7 version now & thanks for taking on the leadership to re-invent the module. Lots can be done with this for sure.

Status: Fixed » Closed (fixed)
Issue tags: -D7 porting

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

Issue tags: +D7 porting

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.