The Volleyball Landesverband Württemberg (VLW) is responsible in organizing official Volleyball competitions in Württemberg, Germany.

They provide game data via XML, thus enabeling interested Clubs to display customized score tabs, results, next game announcements and season schedule on their website.

This module is written primarily for the needs of my local club FV Tübinger Modell, but may be usefull for other Clubs as well.

This module also features integration to the (restricted) XML game data service of the Deutsche Volleyball Liga (DVL) for the 1.Bundesliga and 2.Bundesliga. The submodule for including the new "Dritte Liga" will be included till Sept.2013, since our site depends on it.

Coming up

FIVB and therefore DVL/DVV changed to a new score scheme, all XML interfaces will change, once the dataprovider issue a new xml scheme (scheduled for August 2013).

history of this project

I started a similar module with Drupal5 (only VLW part) but did a rewrite for Drupal6 due to upgrade problems. I needed support for germanwide 2.Bundesliga in 2012, which resulted in restructuring this whole thing into one main and two submodules. Since the inclusion of the germanwide submodule, this project is now interesting for many more clubs, not just my own. This is why I decided to make it public.

Project Page

 git clone --branch 6.x-1.x http://git.drupal.org/sandbox/wingnut00/2021949.git vlw_game_data_service
cd vlw_game_data_service
CommentFileSizeAuthor
#8 coder-results.txt4.49 KBklausi
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

divesh.kumar’s picture

divesh.kumar CreditAttribution: divesh.kumar commented
Status: Needs review » Needs work

Hi,

Please make sure that code sniffer should ran on your code.

Go to this URL and check.

http://ventral.org/pareview/httpgitdrupalorgsandboxwingnut002021949git

divesh.kumar’s picture

divesh.kumar CreditAttribution: divesh.kumar commented

Hi,

Few more comments.

File: vbgame.module
----------------------
Please provide description in .info file along with all

recommended parameters.

In method vbgame_settings_form please wrap renderable strings

like title and description in t() method.

Proper comment blocks are missing everywhere.

vbgame_summary_content() method is way too big around 150+

lines and seems doing everything. Can we break this module in

logical sections?

_vbgame_block_view() have got HTML part in it. Please consider

moving html to template and create a theme hook entry to render

this.

File: vbgame_theme.inc
----------------------
Please move html part from code to templates.

"UPDATE {xxx} SET type='xxx' WHERE type='value'" is not matching drupal's query standards, please change this.

Common suggestions
------------------
Do not use methods like substring rather use drupal_substr or wherever applicable.

cbudzi’s picture

cbudzi CreditAttribution: cbudzi commented

Thanks divesh.kumar for your review!

  • There are still a lot of PAReview.sh Errors, mostly indention issues. I will continue to working on them.

    •
  • I put t() in front of every renderable string I found.
  • All functions should have a doxygen comment by now.
  • I found a solution to vbgame_summary_content():
    there was some code duplication, which I simply transformed into a function.
  • _vbgame_block_view() and vbgame_theme.inc now rely on theme_table(). I didn't know this function before. Thanks!
    This way I probably don't need a template anymore, right?
  • AFAIK there is no db_update() in D6. So how do I do secure UPDATEs?
  • I tried to rid my code from substr, but ran into trubles. Now I replaced most of this code with some preg_match magic.

update

All PAReview issues resolved!

PA robot’s picture

PA robot CreditAttribution: PA robot commented
Status: Needs work » Needs review

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.

divesh.kumar’s picture

divesh.kumar CreditAttribution: divesh.kumar commented

Great! Keep working on..

About "So how do I do secure UPDATEs? ", You would need to write proper drupal sequence or syntax as per drupal query standards and it will be taking care of everything.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Assigned: Unassigned » klausi

I'll look at this now in the Project applications sprint

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
Assigned: klausi » Unassigned
Status: Needs review » Needs work

Sorry for the delay, but you have not listed any reviews of other project applications in your issue summary as strongly recommended in the application documentation.

Review of the 6.x-1.x branch:

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

  1. vbgame.info: description should be a single line and may exceed 80 characters, but not too long.
  2. "variable_set('comment_vbgame_teampage', 'COMMENT_NODE_DISABLED')": all variables defined by your module need to be prefixed with your module's name to avoid name collisions.
  3. "'description' => 'Spielplan Cache',": all descriptive strings in the source code should be in English. Same for "'spielplan' => t('Gesamtspielplan'),", this should be in English, does not make sense otherwise. Please check all your strings.
  4. vbgame_init(): why do you need your CSS on every single page request and cannot add it with the block view or the node view?
  5. _vbgame_block_view(): do not check for access here, that should be handled by the block system.
  6. _vbgame_block_view(): this looks vulnerable to XSS exploits. case 'vbgame_next_promoted_game' directly prints DB data to HTML, and that data stems from an untrusted third party source and has to be considered user provided text, right? Make sure to read https://drupal.org/node/28984 again.
  7. "theme_table($header, $rows, $attributes);": do not call theme_table() directly, use theme('table', ...) instead.

So that potential XSS security issue is a blocker, I don't understand why you did not create a theme function for that with sanitization same as in the other theme functions?

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
CreditAttribution: klausi commented
FileSize
coder-results.txt4.49 KB

Forgot attachment.

PA robot’s picture

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

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

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

PA robot’s picture

PA robot CreditAttribution: PA robot commented
Issue summary: View changes

rephrased

cbudzi’s picture

cbudzi CreditAttribution: cbudzi commented
Issue summary: View changes
Status: Closed (won't fix) » Needs review

Hello,

i finally managed to work on this code again. Thanks klausi for your review!

1) ok, I may have misunderstood the howtos on that...
2) With setting "comment_vbgame_teampage" i'd like to disable comments for this content type by default. Is there another way?
4) my website is using my module for ~90% of its static content, so i'll ship it every time, but I can move it to all view functions if preferred.
5) Ok!
6) all html is now in a theme-function, all variables there are check_plain'd.
7) done.

The current version is adapted to the announced remote changes. I also did a lot of cleanup.

Can you please check again?

pal4life’s picture

pal4life CreditAttribution: pal4life commented
Status: Needs review » Needs work

Hi,
I am just getting in to these reviewing this so began with the Automated Reviewing.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

http://pareview.sh/pareview/httpgitdrupalorgsandboxwingnut002021949git

Please take a look.
Thanks.

PA robot’s picture

PA robot CreditAttribution: 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.