[D7] JasperReports Server SSO

There are some errors reported by automated review tools, did you already check them? See http://ventral.org/pareview/httpgitdrupalorgsandboxccamporeale1979952git

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.

You need to add a README.txt with the description of the module file.

Every contributed module should provide a README.txt in the package. This file should contain a basic overview of what the module does and how someone may use it. The contents of the file may be a repeat of the synopsis on the project page.

hi @ccamporeale

Your branch by default should be the 7.x-1.0-beta1 and you should delete the master branch.

Also you must correct the following notices found in the README.txt file

FILE: /var/www/drupal-7-pareview/pareview_temp/README.txt
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 3 WARNING(S) AFFECTING 3 LINE(S)
--------------------------------------------------------------------------------
1 | WARNING | Line exceeds 80 characters; contains 96 characters
3 | WARNING | Line exceeds 80 characters; contains 217 characters
5 | WARNING | Line exceeds 80 characters; contains 115 characters
--------------------------------------------------------------------------------

It uses http://ventral.org to solve these small bugs

Hi, please change link to git repo:
git clone http://git.drupal.org/sandbox/ccamporeale/1979952.git jasperreports_server_sso

my manual review:

• between first two functions you have two empty lines
• line 156 - undefined constant _SESSION and now it causes notices
• line 153 - unused $account
• line 167 - unused variable $result
• line 172-175:

•   /**
     * Helper functions for the module interface to test the
     * triggersomething trigger.
     */
    

  May be you move these functions into a separate file?

• line 180 - unused variable $arg
• lines 232 - 240: part of description - hook description, so move please, second part - you must use inline comments into functions
• jasperreports_server_sso_blocks_content() - you can use theme fucntion instead
• lines 342 - 344: use drupal_http_build_query() instead.
• line 350 - gramatical error in comment
• line 406 - 423 - You again use 2 comment constructions for one hook. Please move first description in second.

It's only .module file. Please check for such errors in other files yourself.
Good luck.

Hi

Please take a look at the tips for a great project page.

Project automated reviews tools did not find any issues.

Concerning manual review:

• jasperreports_server_sso.module, there's a route 'admin/config/people/jasperreports-server-sso' in the jasperreports_server_sso_menu() that relies on the permission 'administer jasperreports_server_sso settings' in 'access arguments' parameter, but I cannot find a hook_permission() in your module that provides that permission.
• I noticed that you are creating variable $jrs_rest_url several times (see jasper.rest.inc:14, jasper.rest.inc:27, jasperreports_server_sso.module:315) in one way. Why shouldn't you make some function for this? I.e. jasperreports_server_sso_url()?
  

  function jasperreports_server_sso_url() {
    $port = variable_get('jasperreports_server_sso_port', 8080);
    $context = variable_get(
      'jasperreports_server_sso_context',
      'jasperserver-pro'
    );
    return $GLOBALS['base_root'] . ':' . $port . '/' . $context;
  }
  

• What is the goal of using $GLOBALS['base_root'] variable? Why shouldn't you use url() function for this purpose?

Regards

@WebEvt:
Good call on the $jrs_rest_url, I'll add a function for that.
The idea of using $base_root in this case instead of url() is that I only need the host name where drupal and Jasper are running. This function is to craft the proper URI for doing the REST calls to the Jasper API.

Chris,

Attached is a patch that refactors the $jrs_rest_url and uses a function to generate that. If you want to give me access to you repo I can just push it there. I'm keeping this as "needs work" since we need to address the permission hook

- MarianoL

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

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

Update git url