Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Block background module adds background images to block. User can upload background images to each block in block configuration page. We can change the background images of each block and showcase it in site.
Background images can be changed for
1.Newly created blocks.
2.View blocks
3.Core blocks (search,recent comments).
Drupal Sandbox: https://www.drupal.org/sandbox/sarahravikumar/2511202
Drupal: 7.3x
Git Clone Url : git clone --branch 7.x-1.x http://git.drupal.org/sandbox/sarahravikumar/2511202.git block_background
Comment | File | Size | Author |
---|---|---|---|
#15 | pdf_getting_uploaded.png | 164.17 KB | rukayya |
#14 | notice_block_background.png | 154.31 KB | rukayya |
Comments
Comment #1
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedComment #2
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedComment #3
PA robot CreditAttribution: PA robot commentedWe 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.
Comment #4
wolffereast CreditAttribution: wolffereast commentedAutomated Review
http://pareview.sh/pareview/httpgitdrupalorgsandboxsarahravikumar2511202git
When I clone your module using
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/sarahravikumar/2511202.git block_background
I only received an info file. Is the rest of the module yet to be committed?Comment #5
klausiyep, please commit and push your code to git.
Comment #6
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedSorry for the inconvience caused.
I have added the git url for the project.
Comment #7
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedComment #8
das.gautam CreditAttribution: das.gautam commentedHi,
Below points can improve thereview process.
Please follow instructions:
1)Provide correct Git clone Url,Y ou can find the correct git clone command for your sandbox by clicking on the Version control tab, removing the checkbox in front of "Maintainer", and clicking Show. You can then copy-paste the git clone command from the codeblock below "Setting up repository for the first time".
2)If there are any related modules please mention how this module is different.(Recommended)
3)Create a better project page using this reference https://www.drupal.org/node/997024.
Reference : https://www.drupal.org/node/1011698.
Comment #9
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedComment #10
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedHi,
Thank You for pointing me in right direction.
This is a simple module to change the background image of each block in block create page. user can change the image whenever needed in backend.
Comment #11
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedComment #12
das.gautam CreditAttribution: das.gautam commentedThis code should be in install file.
Also in block_background_form_submit(), there is no need to check form_id.
if ($form_state['values']['form_id'] == 'block_admin_configure' || $form_state['values']['form_id'] == 'block_add_block_form')
Comment #13
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedManual Review:
1.Please remove the HTML tag (strong) in description property of .info file
2. In your .info file, remove Information added by drupal.org packaging script on 2015-03-23 and also the lines behind it. It will be automatically added by drupal.org when the project is promoted.
3. Please add an README.txt document for your module. Reference: https://www.drupal.org/node/2181737
4. In your .module file please remove the closing ?> PHP tag and also FILE_REPLACE_EXISTS variable is not defined and it throws some warnings when the uploaded image is altered.
5. We can able to upload any file to the block_background field. Please to restrict to image type files by adding #upload_validators attribute. For example:
Comment #14
rukayya CreditAttribution: rukayya commentedI am getting the Notice Notice: Use of undefined constant FILE_REPLACE_EXISTS - assumed 'FILE_REPLACE_EXISTS' in block_background_form_submit() after enabling and creating a block.
Find the attachment for you info
Comment #15
rukayya CreditAttribution: rukayya commentedI can upload the PDFs as a background which is not acceptable.Find the attachment for the proof.
Comment #16
rukayya CreditAttribution: rukayya commentedplease configure Drupal Coding standard and remove the HTML tags from .info files.
Also your .module file should not be ended with ?>
Comment #17
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedHi All,
Thanks for your support and time for reviewing my module.
Following changes have made:
1. Added upload validators. Only images can be uploaded.
2. Removed unwanted html and php tag at end in module file.
3. Moved file create code in .install file
Comment #18
wolffereast CreditAttribution: wolffereast commentedAutomated Review
pareview.sh shows several minor code standards violations http://pareview.sh/pareview/httpgitdrupalorgsandboxsarahravikumar2511202git
coder found 3 issues though they overlap with items pointed out by pareview
Manual Review
The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.
If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.
This review uses the Project Application Review Template.
Comment #19
ntucakovic CreditAttribution: ntucakovic commentedCODER REVIEW
1) block_background.module#40: String concatenation should be formatted with a space separating the operators (dot .) and the surrounding terms
2) block_background.module#87: missing space after comma
3) block_background.install#93: the final ?> should be omitted from all code files
CODING STANDARDS
Functions are well documented, but hook_help and readme.txt are missing.
Also, project page could be improved.
SUGGESTIONS
block_background.info:
1) You don't need this line: files[] = block_background.module; Every module has to have .module file.
2) Also, you don't need package information. Read more on how to write .info files here:
https://www.drupal.org/node/542202.
Name, core and description are all you need to write here.
3) This module does not make sense if block module is disabled. You should list it as a dependency:
dependencies[] = block
block_background.install:
1) I get this error on uninstall:
Warning: is_dir(): Unable to find the wrapper "publicpublic" - did you forget to enable it when you configured PHP? in file_unmanaged_delete_recursive().
http://oi61.tinypic.com/2hyc3ld.jpg
2) Why do you need hook update?
BUGS:
At first, I could not get this module to work. This was because of these parts of code:
$bid = $block->delta;
and
drupal_add_css('#block-block'.$bid.'
They generated CSS identifier as '#block-block-form', instead of '#block-search-form' that should be generated.
So, when I replaced that parts with the code below, it worked well:
$bid = $block->module . '-' . $block->delta;
and
drupal_add_css('#block-'.$bid.'
Maybe Block module or Drupal were updated in the meantime?
Comment #20
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedHi,
Follwing changes have been made:
1. Readme.txt has been added.
2. Minor code standards issues have been fixed.
3. All the minor bugs have been fixed.
Thank you for reviewing my code. It has helped a lot to improve the module standard.
Comment #21
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedComment #22
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedComment #23
ravi.khetri CreditAttribution: ravi.khetri commentedManual review :
Comment #24
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedComment #25
rrfegade CreditAttribution: rrfegade as a volunteer commentedUse t() function in all the descriptions of .install file
Please update the project page as per the guidelines https://www.drupal.org/node/997024
Comment #26
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedThanks for your comments
I have added t() functions for description in .install file.
Comment #27
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedComment #28
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedHi Sarah Ravikumar,
your application is under regular review process, so should not give the tag "major".
Rolling it back to "Normal"
Comment #29
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedHi sarah ravikumar,
1. Please follow the guidelines of README template.
2. Is there any use of the variable $delete_block_image in block_background.install?
3. You should use the t() function with @ or % placeholders to construct safe and translatable strings.(line 22 of your .install file).
4. Use db_query() to access and modify the values, instead of changing it directly in .install file.
Comment #30
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedHi
Thank you for your comments @ajalan065.
i have made changes in readme.txt .
Also made the changes you mentioned earlier.
Comment #31
ajalan065 CreditAttribution: ajalan065 as a volunteer and at Innoraft commentedHi sarah ravikumar,
1. README looks too short. Add the synopsis, which you have written in your project page.
2. I tried running your module, but encountered this error. http://snag.gy/ZvinA.jpg
Comment #32
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedHi ajalan065,
I cannot replicate the issue which you have posted before.
Can you please uninstall the module and check once.
Thanks,
Sarah.
Comment #33
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedComment #34
ashish.verma85 CreditAttribution: ashish.verma85 commentedHi, Thanks for starting this module.
Few points are:
1. http://pareview.sh/pareview/httpgitdrupalorgsandboxsarahravikumar2511202git
it shows few issues, please fix them
2. it is recommended to add PHP version your .info file.
3. configuration link is missing in your .info file
Comment #35
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedHI,
Thank you for reviewing my code.
I have added the php version.
There is no specific configuration page for this module. Background image is added for each block in its configuration page.
Thanks
Comment #36
ajay_reddyHello
Your module is working fine in Drupal7.
Manual Review
Individual user account
[Yes: Follows] the guidelines for individual user accounts
No duplication
[Yes: Does not cause]
Master Branch
[Yes: Follows]
Licensing
[Yes: Follows]
3rd party assets/code
[Yes: Follows]
README.txt/README.md
[Yes: Follows]
Code long/complex enough for review
[Yes: Follows]
Secure code
[Yes: Meets the security requirements]
Coding style & Drupal API usage
[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:
block_background.install
severity: normalreview: i18n_2Line 22: Use st() instead of t() in hook_install() [i18n_2]
watchdog('system', t('Module install: Attempt to recreate field: "%field", when it already exists.', array('%field' => $field)), WATCHDOG_WARNING);
severity: normalreview: i18n_9Line 22: The $message argument to watchdog() should NOT be enclosed within t(), so that it can be properly translated at display time. [i18n_9]
watchdog('system', t('Module install: Attempt to recreate field: "%field", when it already exists.', array('%field' => $field)), WATCHDOG_WARNING);
Issues found in pareview.sh http://pareview.sh/pareview/httpgitdrupalorgsandboxsarahravikumar2511202git
Comment #37
PA robot CreditAttribution: PA robot commentedProject 1: https://www.drupal.org/node/2540422
Project 2: https://www.drupal.org/node/2511228
As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).
If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #38
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedComment #39
PA robot CreditAttribution: PA robot commentedProject 1: https://www.drupal.org/node/2511228
Project 2: https://www.drupal.org/node/2540422
As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).
If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #40
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedComment #41
Pravin Ajaaz CreditAttribution: Pravin Ajaaz commentedI could see all the recommended changes were made, so looks RTBC to me.
Also fix the few issues reported by pareview before promoting it.
1. In .install file, line 22 (instead of using t(), use get_t() as below):
2. Just leave one new line at the end of the file in .info file. Now there are two new lines.
3. Wrap line 18 in READMe file to make it less than 80 characters.
However these are not blockers, hence changing the status.
Comment #42
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedHi Pravin,
Thank you for reviewing my code.
I will make the changes you discussed before.
Thanks,
Saraswathi.
Comment #43
Saraswathi Ravikumar CreditAttribution: Saraswathi Ravikumar commentedHi,
Please specify the steps to publish this module in drupal.org.
Thank you all for reviewing my code.
Thanks,
Saraswathi.
Comment #44
ajay_reddyHi sarah ravikumar,
After gaining permission to create full project then you can publish this module in drupal.org as full project untill then it will be in sandbox.
Comment #45
PA robot CreditAttribution: PA robot commentedProject 1: https://www.drupal.org/node/2687695
Project 2: https://www.drupal.org/node/2511228
As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).
If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #46
apaderno