GHOP #14: Create a "Drupal for Small businesses" installation profile using only core modules
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | install system |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | patch (code needs work) |
http://code.google.com/p/google-highly-open-participation-drupal/issues/...
This task is to create a Small Business installation profile that can be included with the Drupal core package. A profile lists the modules to be installed and can execute code to do some initial configuration. Having a site that installs with the basic, common pieces in place will make it much easier for people new to Drupal to get started quickly.
This task will require installing Drupal 6 and learning which modules and configurations are required to create a profile which will set up a site with the following pages and the navigation to access them:
* About Us
* Contact Us
* Services Offered
* Latest News
The final deliverable is an issue in the Drupal issue queue with the working profile file attached as a patch.
Resources:
* Macro module (part of Devel module: http://drupal.org/project/devel)
* Other profiles at http://drupal.org/project/Installation+profiles
* Distribution profile group: http://groups.drupal.org/distributions
Estimated time:
2-3 days

#1
Claimed by JBerlinsky (couldn't find d.o username).
#2
#3
Unfortunately, time ran out on this task so it's back up for grabs.
#4
I'm done, the profile is in the attached ZIP file.
#5
Woohoo! Marking for review.
#6
I got the following errors on install on D6beta4:
# The content module is required but was not found. Please move it into the modules subdirectory.
# The profile_generator module is required but was not found. Please move it into the modules subdirectory.
# The watchdog module is required but was not found. Please move it into the modules subdirectory.
Is this profile for Drupal 6? We don't seem to have content and watchdog modules anymore.
Also this was my first time installing from a profile. I didn't realize right away that I needed to create a 'smallbusiness' folder inside profiles and put the profile inside it. Putting the profile directly into the profiles folder allowed me to choose the profile on the install screen and proceed to WSOD. I had to read a lot of install.php before understanding my problem and didn't find any 'install profiles' instructions in the handbook. I guess that this would not happen to others once the profile is properly packaged because it will come inside a folder and include INSTALL.txt.
#7
Sorry! The missing modules were part of some other modules I had installed on my server. I messed with the code and packaged it with INSTALL.txt. Should work now.
#8
Thanks for the INSTALL.txt. I installed the profile on Drupal6beta4 and got just a regular empty site. I don't think the profile was designed for Drupal 6. All the DB changes are in function smallbusiness_profile_final, and _profile_final does not seem to exist in Drupal 6. I found this Drupal 6 profile http://drupal.org/node/144355 for reference of the functions used. If you redo it, you should also pay close attention to your spacing- we try to make the code look very neat: http://drupal.org/node/318
#9
I think I will redo it, maybe I used it on the wrong version :) Thanks for putting up with me.
#10
Thanks a lot, dstelljes. Not too much in terms of the functions you've used have changed in 6.x, so hopefully it'll be a pretty easy port.
#11
Im done if you want me to add more stuff message me.
#12
@ap0c0lyps3: The task description states that your work should be attached as a patch. I didn't download the .zip file but patches should be text files, not .zip files.
If you need some instructions on how to create a patch, check out http://drupal.org/patch/create or you can drop in #drupal-ghop or #drupal on irc.freenode.net
#13
Sorry I'll fix that up. I don't understand how to do it though.
My installation profile is made of completely new files. Doesn't
patches work with files that are already existing?
#14
Ok here is the patch. It only works if you create a blank file to patch on. You
also have to create the folder that the file goes in (sb). You have to run the
patch from that url. Tell me if the patch works. I made it on a windows system
so I'm not 100% sure whether it works. I've uploaded a README file
#15
Hm....since you're only creating new files, a patch isn't entirely necessary, but having one does make it easier to test this because otherwise each file needs to be downloaded individually, then renamed (because d.o renames the file when the file name conflicts with a file that has previously been uploaded). I don't know that you can create the directories in the patch, but you can add new files in a patch (if you're using cvs diff to create the patch, at least). See http://drupal.org/patch/create and look midway down the page for "Adding/Deleting files". If you're not already using cvs to create patches, it really does make things easier. If you're using windows, you probably will want to install cygwin. I think there are instructions on d.o for how to do this.
So, on to the code itself:
sb.patch
< * Installation profile for a small business website. *< 'description' => 'This installation profile sets up a simple Small Business website. '< //Todo: Set settings< //Todo: (optional) Create Small Business theme
< variable_set('site_footer', st('Copyright © 2007 ').$site_name);< //$node = node_submit($node); //Doesn't seem to work using db_query instead to directly insert data< //node_save($node);
This is probably a bad idea. Maybe you can ask for help on IRC with getting node_submit and node_save to work. Doing direct db queries is usually not a great idea when there is an alternative.
< 'body' => st("Welcome to this new Small Business website. You can change this content in the Administration."),"You can change this content from the [link] Administration pages of the site" maybe?
< create_new_node( array( //Creates error pagesIn Drupal we would typically create the array first in this situation, then call the function. Also, no spaces between the '(' of the function call and the first parameter. Also, we'd typically put the comment before the function call (or if..endif loop) rather than within the call.
README.txt
I think I would like to see a page somewhere, probably on the new site itself, that had a link to all of the pages where you put fake information so that the user can make sure to change everything to valid information.
I haven't tested the actual profile yet, but once you get the above things fixed I'll give it a try. I think most of these are pretty minor. If you have questions feel free to respond here or find me (aclight) in IRC in #drupal or #drupal-ghop on irc.freenode.net
#16
Installed cleanly and works fine. Great work. Great contribution!
Suggestions:
(1) name the profile directory itself "small_business" instead of "sb" on the theory that lower skill users will be using this and human readable is better
(2) the contact page would be MUCH better to link to the contact module with a site-wide form instead of just a static page
http://drupal.org/handbook/modules/contact
(3) Suggested tab order: About Us - Latest News - Services Offered - Contact Us
(4) And if you want to blow it out of the park, make the news section a new content type "news" and have the link go to a list of the news items.
#17
Hello.
1. I've attached a patch that creates the directory and file. Inside it you can see which command I used to create it.
2. Also, +1 on calling it small_business in stead of sb.
3. +1 On (2) in #16, using contact module in stead of a static page.
4. There as sill some ^M's in the file. Thus windows line endings. Which editor do you use, maybe I can show you how to remove them from you file.
#18
Thanks for the info guys. I wasn't able to work on this the last couple of days but I will
probably fix all this up today
#19
Thanks for the info guys. I wasn't able to work on this the last couple of days but I will
probably fix all this up today.
@Jax, I actually just downloaded a Windows GUI program from
SourceForge and used it to create a patch. My linux PC is sort of buggered at the moment.
@aclight
I forgot to take those comments out. I was sort of in a rush to finish this thing up. I will fix everything
in the next few hours or so. I am in a internet cafe at the moment
By the way I had to add the node types. The default.profile doesn't create it because it never gets
run. This overrides it and therefore has to do all those things. There seems to be a bug in the
node_save function thats why I wrote it to the database manually
#20
I fixed up everything you guys asked except the Contact form and the news contact
type. I probably will do it tommorow. I uploaded the files as a zip again it seems that
my patching doesn't work all that well. Yes I have converted it to unix line endings
#21
Does anyone know how to create a site-wide contact form with the contact module?
Edit: nvm
#22
So, since this is ready for review I'm guessing that you're not adding the contact form? This should be reviewed as-is?
#23
Now its ready for review. I added every feature you guys asked for: site-wide contact form, latest
news page that lists all news (works with php) etc. I again put it in a zip file instead of
patching. I tested everything and it works 100%.
#24
I tested this with a fresh D6 beta 4 install (I couldn't get HEAD to install for some reason today), and though this is overall good work there are a few small problems:
1. You need to give anonymous users the 'access site-wide contact form' permission so that they can see the "Contact Us" menu link.
2. On your Services offered page, you've used <h3></h3> tags for the headers, however you've also used the Filtered HTML input format for that text field, and h3 is not one of the allowed tags for that input format. You should probably add h1-h6 (or some subset of those) to the list of allowed tags for the HTML input filter settings for the Filtered HTML input format.
3. On your Services offered page, you have the following text:
When writing service descriptions, you should include either prices or e-mails to get quotes, especially to a member of your business in that department. Also add details.','This page is to tell your visitors what services you offer.This looks like a typo to me (near
add details.','This).4. In comment #16 (3) you were asked to rearrange the order of the primary links, but it doesn't look like your order matches the suggested order.
5. In comment #16 (4), you were asked to create a "News" content type and then create a page that only displays nodes of that type. You did as requested, but I'm not sure I like this. Because the task directs you to use only core modules, you can't use Views, which is how one would normally do this. Instead, you have added PHP code into your "Latest News" page that directly queries the database (this is /node/2). This is generally a bad idea. In this case, you're not using db_reqrite_sql (or the D6 equivalent), so there is a chance of this code causing elevation in node access privileges. Also, I don't like the idea of having PHP code in a node for a profile that's targeted to new users in the first place--there's too much potential for things to get messed up accidentally.
I think this would be done better by deleting the news node type, and just adding a vocabulary with some terms. One of the terms could be news. You could then auto-enable the path module by default, and create an alias in the profile that mapped /taxonomy/term/1 to /news, for example. The, just fix the News primary link to point there instead.
Assuming you get rid of the PHP that creates the news node, you can remove php from the list of enabled nodes.
6. As I mentioned in #15 (8) above, I don't like that you are changing the names of a default content type (story). That will be confusing to users as documentation on d.o mentions story types but your users will have blog types instead. You're also changing the description of the page content type.
Now, for the code itself:
1. We don't use ?> at the end of PHP files in Drupal.
2. You still have tabs in several places in your code.
3. The 404 error message page you create needs a period at the end of the text string. Same for the access denied page and the contact us reply page. You also need to pass $contact_form['reply'] and ['category'] through st().
4.
db_query("UPDATE `node_revisions` SET `format` = '3' WHERE `node_revisions`.`vid` =2 LIMIT 1");You need to escape db table names, and get rid of the backticks. You can also take out node_revisions in the WHERE clause since you're only touching one table.
However, I think that if you fix this line:
'format' => '3',by removing the single quotes around the 3 that you won't need to set that via db_query() directly.
5.
VALUES ('%s', '%s', '1', '%s', '%s', '%s', '', '%i', '%s')",%i is not a valid modifier for db_query(). Also, numerical values shouldn't be surrounded by '' in db_query() calls.
I would be more comfortable if you were using an api function to create these nodes instead of direct database calls, also.
#25
Tested from a user perspective (I'm not a coder). RTBC
FANTASTIC. Great basic install profile that should be included in Drupal6. This is an ideal solution for the total noob that needs to figure out how to get a basic site up in Drupal.
#26
I think geilhufe and I might have crossposted, so I'm setting this back to code needs work pending my comments in #24.
#27
Since this is a GHOP task I would suggest to comletely drop the "News" requirement. I think just having a couple of pages and the contact form is already a great install profile. But now I just read that some kind of News page was in the requirement.
What I'm saying is that I don't think it should be made too difficult and too time consuming. Having a basic profile for starters would already be nice. If you're starting with path and terms I think it's already too complicated for a total beginner.
#28
Since the initial requirement did not specify that the news section needs to automatically display all "news" on the site, I'm fine with Jax's suggestion above about going back to having the News page be a simple node.
#29
I already put the news thing in. The code is a bit shaky. So... should I take it out?
@aclight I'll fix those things up tommorrow. Its's already 10pm in my country tommorow.
Why doesn't drupal use the '?>' tag?
It's holiday for me at the moment. I am 14 years old so I sort of also go out alot so
sometimes it takes a day or two before I check up on the comments. Just in case you
wanted to know why I take so long to respond.
#30
The reason some of the MySql queries look so funny is because some of them are
from PhpMyAdmin. I used them because most of my queries weren't working at all.
I tried using the default node functions but they didn't seem to work. The guy who did
the previous version of this task also used the same technique. I think it doesn't work
because everythings not setup yet. Some of the text I got from the previous guys
work. All the php is mine though.
#31
@ap0c0lyps3: I'm not sure why we don't use the ?> tag in PHP files, but I know we don't. As for the SQL queries, the ones you took from PhpMyAdmin aren't necessarily bad, but we try not to escape things when they don't need to be escaped.
#32
@ap0c0lyps3: re: the closing tags, see http://drupal.org/node/545
#33
We haven't heard anything from you recently about this. You're very close here, so I'd like to get this finished so that you can move on to another task.
#34
@aclight: Ok I did everything you asked. For number 5 I removed the PHP code but I never did that
taxonomy thing.
I'm hoping this task is now done. If you find anything I left out just say something. I'll add it.
#35
#36
Ok, this is much closer, but there are still a few minor problems.
1.) When I first installed using this profile, I got these messages when installation was complete. Clearly you have a few variables that need to be initialized ($time) and node array indexes that you need to check for before you use (format, teaser).
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 35.# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 35.
# notice: Undefined index: teaser in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 40.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 40.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 35.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 35.
# notice: Undefined index: teaser in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 40.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 40.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 35.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 35.
# notice: Undefined index: teaser in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 40.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 40.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 35.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 35.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 40.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 35.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 35.
# notice: Undefined index: teaser in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 40.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 40.
# notice: Undefined index: format in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 40.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 35.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 35.
# notice: Undefined index: teaser in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 40.
# notice: Undefined variable: time in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 40.
# notice: Undefined index: format in C:\xampp\htdocs\sb1\profiles\small_business\small_business.profile on line 40.
2.) You still haven't addressed code point #5 in comment #24 (the last point in that comment, regarding %i and numbers in db_query()).
3.) You have some code comments that are not necessary, such as:
//Todo: Change permissions to use site-wide contact formand
//This is the old Latest News page. It was removed on request./*<<<END
<p><?php
$result = db_query("SELECT nid, title FROM {node} WHERE type = 'news'");
while($node = db_fetch_object($result)) {
$result2 = db_query("SELECT body, timestamp FROM {node_revisions} WHERE nid = '".$node->nid."'");
$node2 = db_fetch_object($result2);
$output .= '<div class="node"><h2><a href="'.url("node/".$node->nid).'">';
$output .= $node->title;
$output .= '</a></h2>';
$output .= '<span class="submitted">Submitted on '.date("D, j/n/y", $node2->timestamp).'</span>';
$output .= '<div class="content clear-block">';
$output .= $node2->body.'</div></div>';
echo $output;
$output = "";
}
?></p>
END;*/
Once you've taken care of all of these issues, go ahead and upload your .zip file to the Google issue queue so we can close out this issue.
#37
I have done the things you have asked and uploaded it to the google page
#38
I was told to upload it here as well
#39
So close.....but not quite:
1.) You've now got windows line endings throughout and have a few tabs near the two
'teaser' => '',lines in small_business_profile_tasks().2.) You're still not quite doing your db_query() calls quite right. In create_new_node(), you've enclosed integers in single quotes, which is unnecessary. Also, when you're creating the site wide contact form, you've done this:
db_query("INSERT INTO {contact} (cid,category,recipients,reply,weight,selected)VALUES ('%s','%s','%s','%s','%s','%s')", $contact_form['cid'], $contact_form['category'],
$contact_form['recipients'], $contact_form['reply'], $contact_form['weight'], $contact_form['selected']);
In a few cases, such as for $contact_form['cid'], you're using %s as a placeholder even though you're giving it an integer. That's also true for weight, and probably selected and category (though I'm not sure about category).
Minor point:
I noticed you have this:
$node['body'] = st("You're not authorised to view this page.");I would change the string to "You are not authorized to view this page.", thus removing the contraction and also using American English spellings. If you have other text that does not use American English spellings or uses contractions please fix that as well. It's easier for translators to translate words without contractions and the default Drupal language is American English.
I'm about to head out to the airport right now and I don't have time to test an install to see if you fixed the error messages, but a quick glance at the code suggests that you probably have.
So, again, a few minor things. Please upload your patch here as well to Google. If you can get an updated patch posted within the next 6 hours or so I promise I'll review this by the end of the day (my time) so you can get credit for this. Sorry it's taken so long.
#40
Sorry I took so long to do this. I went out again today. I'm pretty sure everything is done
this time around.
#41
Sorry forgot to mark for review.... again
#42
1. Don't split translatable strings into multi line strings. Your editor will do the line break :-)
2. Read the code style rules...
3. Don't put doubles quotes around anything if not really needed. Change them to single quotes... st()
4. Trailing blank
latest news. "). Aside an candidate for single quotes :-)5.
'node/'.$node['nid'])should be'node/'. $node['nid'])for code style rules.6. "error@error.error" is not an valid mail. Aside "example.com" is the domain for examples...
7.
error_reporting(E_ALL);should be removed on top.8. Please provide patch files...
#43
Ok, I'm still getting error messages when I use this profile. I'm not sure where the node.module errors are coming from, but the teaser ones are coming from creating the error pages, which don't have $node->teaser defined.
* notice: Undefined property: stdClass::$name in /Applications/MAMP/htdocs/sb1/drupal/modules/node/node.module on line 790.* notice: Undefined property: stdClass::$name in /Applications/MAMP/htdocs/sb1/drupal/modules/node/node.module on line 790.
* notice: Undefined property: stdClass::$name in /Applications/MAMP/htdocs/sb1/drupal/modules/node/node.module on line 790.
* notice: Undefined property: stdClass::$name in /Applications/MAMP/htdocs/sb1/drupal/modules/node/node.module on line 790.
* notice: Undefined property: stdClass::$name in /Applications/MAMP/htdocs/sb1/drupal/modules/node/node.module on line 790.
* notice: Undefined index: teaser in /Applications/MAMP/htdocs/sb1/drupal/profiles/small_business/small_business.profile on line 43.
* notice: Undefined index: format in /Applications/MAMP/htdocs/sb1/drupal/profiles/small_business/small_business.profile on line 43.
* notice: Undefined property: stdClass::$name in /Applications/MAMP/htdocs/sb1/drupal/modules/node/node.module on line 790.
* notice: Undefined index: teaser in /Applications/MAMP/htdocs/sb1/drupal/profiles/small_business/small_business.profile on line 43.
* notice: Undefined index: format in /Applications/MAMP/htdocs/sb1/drupal/profiles/small_business/small_business.profile on line 43.
As for Hass's comments, I agree with them for the most part. Patch files would be nice, but I'm not sure if you can create a directory with a CVS patch. If not, don't worry about it. But I guess the task description does say to provide a patch, so if you can do this as a patch that would be ideal.
#44
When I use the profile I don't get those errors. I found the problem though and updated it.
I tried to create a patch but I can't download the software. My cap is up. I will be able to
do it on New Years though
#45
This type of syntax is NOT allowed in t() or st(). Change this to a one liner and remove the "\n". Additional do not make line breaks inside your SQL code. We have good editors that break the line them self if required.
Wrong:
'body' => st('This page is to tell your visitors what services you offer.\n\n'.'<em>Sample Service</em>\n'.
'Services can be separated by headings, as shown above.\n\n'.
'<em>Sample Service 2</em>\n'.
'When writing service descriptions, you should include either'.
' prices or e-mails to get quotes, especially to a member of'.
' your business in that department. Also add details.'),
Correct:
'body' => st('This page is to tell your visitors what services you offer. <em>Sample Service</em> Services can be separated by headings, as shown above. <em>Sample Service 2</em> When writing service descriptions, you should include either prices or e-mails to get quotes, especially to a member of your business in that department. Also add details.'),Wrong:
variable_set('site_footer', st('Copyright © 2007 ').$site_name);Correct:
variable_set('site_footer', st('Copyright © 2007 %site_name', array('%site_name' => $site_name)));#46
#47
#48
It looks to me that you've addressed all of hass's complaints.
So, hopefully you can download the software so that you can make this into a proper patch.
In addition, I see two very minor issues:
1. It's 2008 now, so you might as well add that as the copyright date.
2. There's an extra space after the period in the following line:
'description' => 'This installation profile sets up a simple small business website. 'I have not tested this most recent version to see if it actually installs without an error. Please test yourself before submitting a patch so that you catch such things.
So, for this task to be complete, you'll need to fix the things above.
Separate from this GHOP task, if you want this installation profile to be easy to find for other users, it will need to be created as an official project on drupal.org. For that to happen you'll either need to apply for CVS access on d.o or talk with someone who does have CVS access and ask them to be the official maintainer for the installation profile. Just to be clear, finding a maintainer isn't at all required for you to complete the GHOP task, it's just something to think about in the future.
#49
1. Shouldn't we remove the following unneeded code? Makes no sense to me to keep this and will reduce file size.
/*** Return a list of tasks that this profile supports.
*
* @return
* A keyed array of tasks the profile will perform during
* the final stage. The keys of the array will be used internally,
* while the values will be displayed to the user in the installer
* task list.
*/
function small_business_profile_task_list() {
}
2. Is menu_rebuild(); really required? I haven't tested this yet, but this is something setup/upgrade process does them self... i only don't know if it does this in this after the profile or before.
3. 'Thank you for contacting us. We will contact you shortly' is mission a period.
4.
WHERE rid = '1'andWHERE rid = '2'shouldn't use single quotes. The single quote causes a string to integer conversion on DB and this should be avoided. 1 and 2 are integers and should be added as integer... :-)5.
"Contact Us"- is this correct English spelling? I feel like it should be"Contact us"... but i'm not a navtive speaker and maybe wrong.6. Code style
/*Creates error pages*/should be// Creates error pages.7. this:
/*** Installation profile for a small business website.
*/
//Project Page: http://drupal.org/node/197235
...should be written as:
/*** Installation profile for a small business website. Project page: http://drupal.org/node/197235
*/
8. You should place a blank after
//like// Foo.Sorry for going to be nitpicking. I'm stopping here for now.
#50
From the notices above i think there are missing array values on the error pages... this is not fixed in latest code as i can see. See after /*Creates error pages*/ - i cannot see teaser, name and format in this array. Maybe the source.
Why are you using here a different array syntax as before? This should be synced... aside the missing array values would cause the warnings here.
$node['nid'] = 6;$node['vid'] = 6;
$node['title'] = st('Access Denied');
$node['body'] = st('You are not authorized to view this page.');
Why not using foreach for the node creation as above code for node type and so on does, too?
#51
@aclight, I think the idea was for this to be a patch against Drupal core for possible future inclusion with core and not as a separate contributed Install profile. Dmitiri is the owner of the task so I guess he and ap0c0lyps3 can discuss if they want to make this a project on d.o for 6 as well (which sounds like a good idea since this definitely won't ship with 6.)
#52
Happy new year everybody!!!!! Hope you guys had a great time with your friends last night.
Anyway back to business. I updates those things you guys asked for. I again wasn't able to
test it.
@add1sun I'm not sure what you mean by d.o and all that (I'm sort of new to the drupal community)
But this project is already for Drupal 6. I haven't tested it on anthing below
#53
I'm sorry for not bringing this up earlier, but i overlooked it. I would make the year dynamic:
st('Copyright © @year @site_name', array('@site_name' => $site_name, '@year' => date('Y'))));I'm not sure if the theme placeholders are a good idea here... therefor changed to
@. Everything else looks good now.#54
Ok added your update. I can't believe that I forgot that as well :D. So it looks like this task is done?
#55
Ugh sorry windows line ends
#58
re: #52, d.o = drupal.org. I was referring to the fact that this patch will not go into D6 core since we are late in that process so, for core, this patch will be moved to D7 (and have to be updated for that later.) In the meantime since this is written for D6, you and dmitri may want to make this into a contributed profile ("project on d.o") so people can use it for the D6 release, but that is outside of scope of the task itself and up to whether dmitri (or someone else) wants to take on the responsibility of maintaining it.
#59
It would be best if you would test your patches before you post them. You will get quicker reviews if your patches are already tested, because you will find the errors yourself that we are pointing out and you can then fix them before posting your patch. For example, your latest patch has several syntax errors that prevents it from even being parsed correctly. In line 152 you are missing a comma at the end of the line:
'type' => 'page'Lines 157-164 use the wrong syntax for creating an array, and also use semicolons instead of commas at the end of the lines.
<?phparray(
'nid' = 6;
'vid' = 6;
'teaser' = '';
'format' = 1;
'title' = st('Access Denied');
'body' = st('You are not authorized to view this page.');
),
?>
should instead be
<?phparray(
'nid' => 6,
'vid' => 6,
'teaser' => '',
'format' => 1,
'title' => st('Access Denied'),
'body' => st('You are not authorized to view this page.'),
),
?>
After fixing all of these parse errors, and installing using this profile, there are still error messages after installation:
* notice: Undefined index: type in /var/www/sb3/drupal/profiles/small_business/small_business.profile on line 19.* notice: Undefined index: status in /var/www/sb3/drupal/profiles/small_business/small_business.profile on line 19.
* notice: Undefined index: promote in /var/www/sb3/drupal/profiles/small_business/small_business.profile on line 20.
* notice: Undefined index: sticky in /var/www/sb3/drupal/profiles/small_business/small_business.profile on line 20.
I also noticed that you still have not done the following things mentioned by myself or other reviewers previously:
A.) Make this an actual patch file instead of a .zip file.
B.) Fix spacing issues throughout the file. In many places you don't have spaces after commas in a series (eg.
%d,%d,'%s','%s',%d,%d,%d,%d) and your comments don't have appropriate spacing (eg. the block comment at the top is missing spaces at the beginning of the 2nd and 3rd lines, and in many cases your comments look like//This is incorrectinstead of// This is correct.).And additional things I noticed that are new or things I had not noticed before:
C.) In the array of error page nodes, you shouldn't have the nested arrays both on the same line as you do in line 145:
$nodes = array( array(Instead, you should nest them just as you had just above that point where you are creating the other nested array of nodes.
D.) On line 119, the body of your Services Offered node, you use the following text as the body:
'body' => st('This page is to tell your visitors what services you offer. <em>Sample Service</em> Services can be separated by headings, as shown above. <em>Sample Service 2</em> When writing service descriptions, you should include either prices or e-mails to get quotes, especially to a member of your business in that department. Also add details.'),You are not using headings here, so the text doesn't make any sense.
So, as I have said before, you're very close to completing this task, but you have to make sure that your patch is a patch and that it works as you expect it to before you post it for review by others. Please take the time to test it yourself. The fewer times we have to tell you your patch needs work the less likely it will be that we will find minor imperfections like spacing issues, etc. that we also want you to fix :)
#60
I saw you broke a feature in your last patch...
variable_set('site_footer',is now missing... earlier patches were ok... and the string with year does "nothing".#61
I'm struggling to create a patch. I always get an error something about not being able to obtain a lock
on the small_business folder. I'm sorry I wasn't working the last few days. I was on holiday. I'll post the
latest (tested) update in half an hour
#62
While I try and figure out how to create a patch with folders...
This one has been tested with HEAD this time. I used to use Drupal 6 (not RC1).
#63
While I try and figure out how to create a patch with folders...
This one has been tested with HEAD this time. I used to use Drupal 6 (not RC1).
#64
#65
First, the good news:
1. No errors on installation! Yay!
2. Footer with copyright date works.
And now, the bad :(
A.) Again, this file has windows line endings. What editor are you using? We can probably help you find the setting to not use windows line endings so you don't keep having this problem.
B.) You've fixed the whitespace issues in most places, but there's still one line that needs spaces after the commas in a series (line 18):
function create_new_node($node) {db_query("INSERT INTO {node} (nid,vid,type,title,uid,status,created,changed,comment,promote,moderate,sticky) VALUES (%d,%d,'%s','%s',%d,%d,%d,%d,0,%d,0,%d)",
C.) dmitrig01 had a good point at http://code.google.com/p/google-highly-open-participation-drupal/issues/... about naming create_new_node to small_business_create_new_node and likewise with create_primary_link. I don't understand your response to him about that in comment #38.
D.) You still haven't fixed the Services offered page as I mentioned in comment #59D.
Lets get these fixed, roll a patch, and call it a day.
#66
I don't understand what you meant at #59D thats why I didn't put it in. I fixed all of those things up (except #59D) but I'm sort of stuck with creating a patch. Can you help me with that? I followed the instructions at http://drupal.org/patch/create but when I create the patch it doesn't work properly
#67
Here's what I wrote in comment #59D:
I am fine with the first and last sentence, starting with "When writing service descriptions.....". It's the second sentence that doesn't make any sense to me. You are no longer using headings on that node, yet you say "as shown above". The easiest thing to do is just delete the following:
Re: creating patches:
The easiest thing is for you to join the #drupal-ghop IRC channel on irc.freenode.net and ping me (aclight), or if I don't happen to be there, just ask if someone can guide you through creating a patch. If there is nobody in that channel you can try #drupal. Just say you're a GHOP student working on a task and you need help creating a patch.
If you won't or can't use IRC, then you'll need to be more explicit and explain what trouble you are having when creating your patch. What are you trying to do? What tools (programs) are you using to create the patch and what commands are you giving them? What platform are you using (I assume Windows since many of your files have windows line endings)?
If you're able to create a patch but it isn't right, upload it here with an issue and just point out in the comment text that this patch has problems and explain how you tried to create it.
But of course, we'll try to help you figure out what's going wrong with your patching if you can give us a place to start!
#68
link
#69
reverting proper title
#70
Fixing the title that got changed.
(crossposted w/westwesterson)
#71
In the meanwhile... I should make a patch pretty soon.
#72
Actually, don't worry about a patch. You've finally gotten the code itself to the point where I have no complaints, and this is going to be a really tricky patch to create in the first place.
Good job on this. I'll mark this task as closed in the Google tracker so you get credit for it. Please make sure to upload your final .zip file to the google task as well so you will officially get credit.
Great job!
#73
New features go to Drupal 7. Thanks for your work.
#74
Rolling the patch is trivial, unless I missed something. The attached patch contains the code from #71. I have not tested it, and it is likely to require updating for Drupal 7 compatibility.
#75
At least one thing is replacing "Story" with "Article".
#76
Why simply core modules only? I think CCK, Views, and Webforms would be indispensible here.
#77
@gh0st25: the idea is that this would be a profile that would ship with Drupal core. Thus, it must use only Drupal core modules.
#78
Here's an update. I ran it through the code-fixer, repasted the current node type descriptions from default.profile (fixing s/story/article), and also got rid of the news node type. It doesn't seem to do anything now that the article node type isn't doing.
There's one problem, which is that most businesses would prefer their news to be on a separate page rather than the home page, and have the home page instead contain some welcome text and introduction. This is easy to fix by making the home page something other than "node" and then linking "node" as a News tab. Optionally with neat URL aliases. Perhaps that should go in.
#79
Thanks Arancaytar!
There's some odd English in here ("Oops!! It looks as the page you are a looking for no longer exists or your URL is incorrectly formatted.") and a few missing periods and stuff but I like fixing stuff like this. I'll give this a run-through tomorrow and post a new patch based on your great efforts here.