Havin' a crack at getting started on TODO things.

// TODO Please convert this statement to the D7 database API syntax.

Going to get all the .install file queries ported over to the D7 db API syntax

CommentFileSizeAuthor
#21 storm-1798036-install_sql_syntax-21.patch3.9 KBwillwh
#19 storm-1798036-install_sql_syntax-19.patch3.9 KBwillwh
#17 1798036-database_api_updates_install-17.patch7.96 KBwillwh
#14 storm-d7-api-updates-1798036-13.patch7.79 KBwillwh
#11 storm-d7-api-updates-1798036-5.patch3.81 KBwillwh
#10 storm-d7-api-updates-1798036-5.patch3.81 KBwillwh
#8 storm-d7-api-updates-1798036-5.patch12.18 KBwillwh
#6 storm-d7-api-updates-1798036-5.patch3.79 KBwillwh
#4 storm-d7-api-updates-1798036-0.patch3.79 KBwillwh

Comments

willwh’s picture

willwh commented
Title: stormproject.install - // TODO Please convert this statement to the D7 database API syntax. » storm*.install - // TODO Please convert statements to the D7 database API syntax.

This looks ok.

db_delete('stormattribute')
  ->condition('domain', array('Project status', 'Project status search', 'Project category', 'Project category search', 'Project priority', 'Project priority search', 'Duration unit'), 'IN');
juliangb’s picture

juliangb commented

Are you able to post your code as patches please?

It will make it easier for us to review and commit them more quickly...

willwh’s picture

willwh commented

Hi Julian,

After chatting with kfritsche in IRC, I was going to hold off until I've patched all the queries in the .install files.

I will definitely be creating a patch when ready.

willwh’s picture

willwh commented
StatusFileSize
new storm-d7-api-updates-1798036-0.patch3.79 KB

Well, here we go, my first crack at a patch :)

git branch 1798036-database_api_updates_install
git checkout 1798036-database_api_updates_install

// made my changes

git add -A
git commit -m "some comment"
git diff 7.x-1.x > storm-d7-api-updates-1798036-0.patch

How'd I do?

kfritsche’s picture

kfritsche
Primary language German
Location 🇩🇪🇪🇺
commented
Status: Active » Needs work 
+++ b/stormtask/stormtask.installundefined
@@ -73,7 +73,11 @@ function stormtask_install() {
-  db_query("UPDATE {stormattribute} SET isdefault = 1 WHERE domain = 'Task category' and akey = 'task'");
+  db_update('stormattribute')
+    ->field('isdefault', 1)
+    ->condition('domain', 'task category')
+    ->condition('akey', 'task')
+    ->execute();
 }

I think this should be:

    ->fields(array('isdefault', 1))

Yeah, I know I told you otherwise before.

And the indent is not correct.
Otherwise it seems okay for me.

willwh’s picture

willwh commented
Status: Needs work » Needs review
StatusFileSize
new storm-d7-api-updates-1798036-5.patch3.79 KB

Ok - here goes :)

Thanks for the pointers kfritsche!

Status: Needs review » Needs work

The last submitted patch, storm-d7-api-updates-1798036-5.patch, failed testing.

willwh’s picture

willwh commented
Status: Needs work » Needs review
StatusFileSize
new storm-d7-api-updates-1798036-5.patch12.18 KB

I should remember to git add & commit before making a patch ;)

Status: Needs review » Needs work

The last submitted patch, storm-d7-api-updates-1798036-5.patch, failed testing.

willwh’s picture

willwh commented
StatusFileSize
new storm-d7-api-updates-1798036-5.patch3.81 KB

Ok, without commit the patch & fixing indents ;)

willwh’s picture

willwh commented
Status: Needs work » Needs review
StatusFileSize
new storm-d7-api-updates-1798036-5.patch3.81 KB

Ok - should be good to go now, I hope :)

Status: Needs review » Needs work

The last submitted patch, storm-d7-api-updates-1798036-5.patch, failed testing.

kfritsche’s picture

kfritsche
Primary language German
Location 🇩🇪🇪🇺
commented

Seems okay now. Tests will fail, because tested components doesn't work yet.

Good work. Keep it up!

willwh’s picture

willwh commented
Status: Needs work » Needs review
StatusFileSize
new storm-d7-api-updates-1798036-13.patch7.79 KB

stormtask had a SQL syntax error in the install - only after uninstall / installing this module did I notice it. Fixed it up in this patch.

Going to move on to fixing up some different stuff now... progress continues :)

juliangb’s picture

juliangb commented
Title: storm*.install - // TODO Please convert statements to the D7 database API syntax. » storm*.install - convert statements to the D7 database API syntax
Status: Needs review » Needs work

I'm pretty sure we need an ->execute() for the delete commands to work...

juliangb’s picture

juliangb commented

https://drupal.org/node/310081 might help.

willwh’s picture

willwh commented
Status: Needs work » Needs review
StatusFileSize
new 1798036-database_api_updates_install-17.patch7.96 KB

This should do the trick.

juliangb’s picture

juliangb commented
Status: Needs review » Needs work

Patch needs unix line endings.

willwh’s picture

willwh commented
Status: Needs work » Needs review
StatusFileSize
new storm-1798036-install_sql_syntax-19.patch3.9 KB

Ok, here we go :)

Status: Needs review » Needs work

The last submitted patch, storm-1798036-install_sql_syntax-19.patch, failed testing.

willwh’s picture

willwh commented
StatusFileSize
new storm-1798036-install_sql_syntax-21.patch3.9 KB

Trying again :)

juliangb’s picture

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

juliangb commented
Status: Needs review » Fixed

Committed, thanks!

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

(not verified) commented
Issue summary: View changes

Should cover all sub module install files