The module extends Webform by adding a Computed Component which can receive a value from either a stand-alone mathematical calculation, a SELECT-CASE-DEFAULT value assignment, or from one of an IF-THEN-ELSE mathematical calculation/assignment.

No user programming required to create calculations etc., although the module depends on PHP Filter for use of drupal_eval.

Webform Computed Component sandbox

D7 git branch:
git clone --branch 7.x-3.x http://git.drupal.org/sandbox/vernond/1301484.git webform_computed_component

D6 git branch:
git clone --branch 6.x-3.x http://git.drupal.org/sandbox/vernond/1301484.git webform_computed_component

Comments

patrickd’s picture

patrickd commented

welcome

There are still files other than README.txt in the master branch, make sure to remove them. See also step 5 in http://drupal.org/node/1127732
Review of the 7.x-3.x branch:

  • Remove LICENSE.txt, it will be added by drupal.org packaging automatically.
  • The "?>" PHP delimiter at the end of files is discouraged, see http://drupal.org/node/318#phptags
    ./webform_computed.module
  • ./webform_computed.module: all functions should be prefixed with your module/theme name to avoid name clashes. See http://drupal.org/node/318#naming
    False positives
    function _webform_defaults_computed() {
function _webform_theme_computed() {
function _webform_edit_computed($component) {
function _webform_render_computed($component, $value = NULL, $filter = TRUE) {
function _webform_display_computed($component, $value, $format = 'html') {
function _webform_analysis_computed($component, $sids = array()) {
function _webform_table_computed($component, $value) {
function _webform_csv_headers_computed($component, $export_options) {
function _webform_csv_data_computed($component, $export_options, $value) {
function webform_components_computed_validate($element, $form_state) {

  • Run coder to check your style, some issues were found (please check the Drupal coding standards). See link.
  • Drupal Code Sniffer has found some code style issues (please check the Drupal coding standards). See link.

The link to the automated review: http://ventral.org/pareview/httpgitdrupalorgsandboxvernond1301484git (you can use this site to re-test your self)
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. Go and review some other project applications, so we can get back to yours sooner.

Is there a reason for naming your branches 3.x? this is quite confusing as you never had a 1.x/2.x

Please format your commit messages well, at least if they have issues (See http://drupal.org/node/52287)

dependencies[] = php
Good joke ;D you can remove that, (please)

regards

vernond’s picture

vernond commented

Thank you for the prompt review! Issues raised are listed below with actions and/or explanations:

  • Files other than README.txt in the master branch - extraneous files removed and README.txt updated, committed to both branches
  • Remove LICENSE.txt - removed from both branches
  • The end PHP block delimiter at the end of files is discouraged - the closing tags feature in dynamically created code sent to php_eval() as per requirement
  • all functions should be prefixed with your module/theme name to avoid name clashes - the functions listed are hooks that Webform module exposes to permit external modules to add components to Webform
  • Run coder to check your style - done and committed fixes to both branches (with the exception of drupal_eval warnings on D6 branch)
  • Drupal Code Sniffer has found some code style issues - issues handled and committed to both branches ( see D7 report at http://ventral.org/pareview/httpgitdrupalorgsandboxvernond1301484git and D6 report at http://ventral.org/pareview/httpgitdrupalorgsandboxvernond1301484git-6x-3x)
  • Is there a reason for naming your branches 3.x (no 1.x/2.x)? - The reasoning is to indicate which versions of Webform this module is tracking
  • Please format your commit messages - I was lax on commit message formatting rules in the sandbox, new commits feature issue numbers etc. as required
  • dependencies[] = php you can remove that - Core's php.module is a requirement for use of php_eval().
vernond’s picture

vernond commented

The point you raise re the dependency on PHP Filter merits some thought. It is not ideal that PHP filter needs to be enabled to utilise this Webform component (nor is it ideal to complicate the issue with all sorts of access permissions).

To circumvent this I shall test a change where we instead module_load_include() the core php.module file in order to have access its to php_eval() function. From a security standpoint this seems to satisfactorily reduce risk as PHP filter need not be enabled and eval'd code will remain effectively jailed. Also means that I do not need to do anything dodgy like duplicating php_eval within the module.

Will post back when I have had time to try out this change.

vernond’s picture

vernond commented

Dependency on core PHP module (PHP Filters) removed from 7.x-3.x branch (README.txt updated also).

No change to 6.x-3.x branch as drupal_eval() function is in common.inc, thus no dependency was required.

Coder and DrupalCS tests were re-run and resolvable issues handled.

patrickd’s picture

patrickd commented

You still have not removed the php closing tag
The "?>" PHP delimiter at the end of files is discouraged

Follow/subscribe your issue queue. You got some feature requests, please answer them.

We ad the webform version issue several times here and it was the most common way to ignore webforms version number and begin with 1.x

As my webform API knowledge is quite limited I've to stop on this state, sorry

regards.

vernond’s picture

vernond commented
Status: Needs review » Needs work

Thanks a bundle for your input so far Patrick!

I cannot remove the closing tags as they are required by Drupal core's PHP module for eval'd code. Without eval'd code this module cannot offer anything meaningful. The closing tags that are being identified are in text strings, not at the end of the module file. It's a false positive.

I do subscribe/follow my issue queue. Two of the three issues on the queue were raised by me as reminders for things to do when other commitments back off a little. The other issue is a known sticky point in Webform itself and I'm hoping to solve that issue for both projects (again, as time allows).

Setting to 'needs work' until I get version number fixed.

misc’s picture

misc commented

@vernond has been contacted to ask if the application is abandoned.

After ten weeks with a status of needs work: the applicant may be contacted by a reviewer to determine whether the application was indeed abandoned. The action taken by the reviewer should be documented in the project application issue.

http://drupal.org/node/894256

vernond’s picture

vernond commented
Status: Needs work » Closed (won't fix)

This sandbox project and its project application are abandoned in favour of a new sandbox project at http://drupal.org/sandbox/vernond/1667748