Drupal.org
Issues

Onki

Posted by teelmo on May 28, 2012 at 1:10pm
Project:Drupal.org Project applications
Component:module
Category:task
Priority:normal
Assigned:Unassigned
Status:needs work

Issue Summary

Onki module integrates Onki ontology services into Drupal taxonomies.

Onki

About Onki

The ONKI service contains Finnish and international ontologies, vocabularies and thesauri needed to publishing your content with support for linked data. Ontologies are conceptual models identifying the concepts of a domain. They contain machine "understandable" descriptions of the relations between the concepts.

ONKI is published and currently maintained by Semantic Computing Research Group SeCo. The Finnish Government is also involved in the development and future proofing - press release in Swedish, Finnish and English (Google translate).

Onki module

Onki Drupal module integrates Onki service with Drupal taxonomies. This way you can use Onki terms to describe (tag) desired content types.

Preparations

To set up ONKI module you have to have at least one content type (i.e. article) and a taxonomy (i.e. tags) configured in your system.

To create a content type browse to:
/admin/structure/types/add

To create a taxonomy browse to:
/admin/structure/taxonomy/add

To use this module you need to register an API key for Onki service. The API key is per IP address.

Install and configure

1. Download or clone ONKI module from Sandbox:

2. Enable ONKI module:

  • /admin/modules

3. Configure ONKI module's settings:

  • /admin/config/content/onki

There are many settings options, but the example values provided work in most cases.

Onki admin settings

After you have set up all the settings you can try the service. If you attached Onki to the article content type you browse to:

  • /node/add/article

You should see an Onki field which allows you to add new terms. The autocomplete feature should kick in when you start to type in your terms. If not, check your settings and specially your language and API key settings.

Onki user interface

After you save your article you see the Onki URL, not the human-readable title. You can fix this by managing content type's display. So for example if you are using the article content type you browse to:
admin/structure/types/manage/article/display

where you can select "Onki field" as the format. You should do this for both default and teaser display modes.

Thats it! Congratz!

Other information

This module was developed & sponsored by svenska.yle.fi (part of Yle - Finnish Broadcasting Company). The developers are Teemo Tebest (Yleisradio) and Tomi Mikola (Wunderkraut). The module is currently used in production by Svenska Yle (Launch april 2012).

Onki module makes use of CSS3.

Project's sandbox page:
http://drupal.org/sandbox/teelmo/1596454

Git repository usage:
git clone teelmo@git.drupal.org:sandbox/teelmo/1596454.git onki

The above command should be run for example in your {drupal_root}/sites/all folder.

Drupal Version:
Drupal 7.x, tested with 7.17

Login or register to post comments

Comments

#1

Posted by teelmo on May 28, 2012 at 1:17pm
Status:active» needs review

#2

Posted by igoen on May 28, 2012 at 3:31pm
Status:needs review» needs work

Hi,
I do automated project review, and there's many errors of coding style, you should check it in ventral.org.
And after I install it, there something collapsed. See screenshots.

Sorry I just review for layout, and for module work I'm still looking up.
( I'm also beginner in drupal.)

AttachmentSize
Collapsed.png 61.13 KB

#3

Posted by teelmo on May 30, 2012 at 6:54am

Thanks for the review.

I will look into these as soon as I have the time.

(I did the coding style check with coder module but I will look into that ventral.org too)

#4

Posted by dman on August 23, 2012 at 1:36pm

I did a line-by-line visual and test review with teelmo @ drupalcon today.

The utility this provides is useful, appropriate, and leverages and provides a useful service. It's a good thing to have.

Suggested that exactly *what* it does and why you'd want it should be added to the top of the project page above the technical details. (screenshot or diagram?)

No direct conflict with other modules, but do compare with (and add a link to cross-reference to) web_taxonomy project which has a number of similarities thought the method and architecture is different.

* minor indentation cleanups recommended - todo
* an amount of redundant unused code (functions from earlier attempts & SOAP stuff) to remove (hook_install & the URI funcs).
* old 'features' inc files to remove.

* Configuration settings page - very good and clear.
* Proper use of Drupal APIs throughout : 90% (some DB queries could be DBTNG, but they are not wrong)
* No apparent security issues
* well structured, well commented code.

However
* didn't actually work on a fresh install. That should be fixed offline.

Make it *work* and get a clean report from Ventral, and I'd say the *code* review is done and it should be RTBC.

.dan.

#5

Posted by teelmo on August 31, 2012 at 11:47am

Thx dman awesome!

I'll get on with these changes a soon I have the minute!

#6

Posted by teelmo on November 12, 2012 at 12:14pm

Made Coder review and fixed some error regarding punctuation. Now returning:
Coder found 1 projects, 6 files, 0 warnings were flagged to be ignored

Also made a review in Ventral.org and fixed some more things based on it.
http://ventral.org/pareview/httpgitdrupalorgsandboxteelmo1596454git-7x-1x

Now both reviews say that the code is clean.

Also updated the documentation and installation instruction and fixed some bugs that caused a clean install to fail. Now tested that clean install works. Also removed the master branch and started using the version specific branch instead.

#7

Posted by teelmo on November 12, 2012 at 12:22pm

Duplicate.

#8

Posted by teelmo on November 12, 2012 at 12:41pm
Status:needs work» needs review

Needs someone to verify that clean install works and the module works.

#9

Posted by mohs3n71 on November 28, 2012 at 9:23pm

i think you should remove lines that you make them comment in development
like this one in line 141 in .module file

        // '#required' => TRUE,

and also in line 191
      // $matches["<create>$tags"] = t($string, array('%term' => $tags));

it's better that you user @return to explain about return type and object of a function
for example in lines 526 to 531

/**
* Helper function for testing Onki service.
*
* Helper function to test if ONKI service is up and runnig or not.
* Return TRUE if ONKI is running and FALSE if a timeout occurs.
*/

if you fix theese your code is great

thank you ,
Mohsen

#10

Posted by mohs3n71 on November 28, 2012 at 9:29pm
Status:needs review» needs work

#11

Posted by teelmo on January 2, 2013 at 11:46am
Status:needs work» reviewed & tested by the community

I fixed those three things. Now committed to head. Changed status to review and tested by the community.

#12

Posted by klausi on January 3, 2013 at 11:29am
Status:reviewed & tested by the community» needs review

Don't RTBC your own issues, see http://drupal.org/node/532400

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

#13

Posted by robinvdvleuten on January 3, 2013 at 4:49pm
Status:needs review» needs work

There are still some issues with the automated PAReview:
http://ventral.org/pareview/httpgitdrupalorgsandboxteelmo1596454git

Manual review:

  • How does Drupal handle adding a different version of the jQuery UI? In my experience it gives a lot of strange errors.
  • It is recommended to use db_select instead of db_query on line 287 in onki.module.
  • In your CSS I see some CSS3 prefixes, maybe you can make a notice somewhere which browsers are supported.

#14

Posted by teelmo on January 7, 2013 at 7:53am

Sorry about RTBC'n my own issue :|

#15

Posted by teelmo on January 7, 2013 at 8:09am

Fixed the issues given by the automated PAReview. There were some CSS style's styling issues and couple of comment's styling issues. Also changed join() function calls to implode().

  • I don't have knowledge about different jQuery UI versions, but the jQuery UI is actually redundant at this stage so its gone.
  • I don't know if any of the conditions described here apply in this case.
  • CSS3 features downgrade gracefully in this case but I'll add a comment that works best with CSS3 supported browsers.

#16

Posted by teelmo on January 7, 2013 at 8:17am
Status:needs work» needs review

Changing project status.

#17

Posted by robinvdvleuten on January 14, 2013 at 10:49am
Status:needs review» needs work

Hi,

I see that you have a 'onki.features.field.inc' file in your repo. What's the purpose of this file? I don't see any references to the features module or usage of this file.
Why do you set a status message after uninstalling your module? When the uninstallation process doesn't throw any errors, it is very obvious that it is correctly uninstalled.
If db_query is better than db_select is a point of discussion, but for now I do not see any real issues here after reviewing the query.

#18

Posted by PA robot on March 25, 2013 at 5:25pm
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.

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

#19

Posted by jaysire on May 24, 2013 at 11:02pm
Status:closed (won't fix)» needs review

Reopened after fixing the following:

* Unnecessary file removed
* Code cleaned to be compliant with ventral.org
* Drupal_set_message removed from uninstall hook
* Spelling errors fixed and comments cleaned

#20

Posted by klausi on May 24, 2013 at 11:56pm
Status:needs review» needs work

Reviewed this with jaysire at Drupalcon.

manual review:

  1. onki_enable(): that can be removed because yxou can use default values with variable_get().
  2. "'access arguments' => array('create article content'),": you should probably use your own permission instead?
  3. "'#autocomplete_path' => 'onki/autocomplete/1',": where does the "1" come from? Should still work without it?
  4. "if (module_exists('arpa')) {": that seems like an internal module, so you should add a comment.
  5. $form['onki_dialog']['onki_dialog_btn']: do not use custom markup, use the submit type of the form API. See http://api.drupal.org/api/drupal/developer!topics!forms_api_reference.html/7#submit
  6. onki_form_after_build(): docs are wrong, this is not a hook. See http://drupal.org/node/1354#functions
  7. onki_form_after_build(): i think you don't need that function, just use #attached for JS and CSS in the form building function.
  8. onki_autocomplete(): you are loading user supplied text from the term table, so you need to sanitize it before printing to avoid XSS. Please read http://drupal.org/node/28984
  9. onki_search(): most function docs do not tell me anything, you should improve that.
  10. "htmlspecialchars($r['label'], ENT_COMPAT, 'UTF-8');": use check_plain() instead.
  11. onki_search(): you should not sanitize here, but in the function that actually prints the data to the user (in this case it is onki_autocomplete()).
  12. onki_json_concepttree(): I'm not sure where the tree descriptions are sanitized? You can do that either in Javascript or in PHP before delivering the ajax results.
  13. onki_taxonomy_term_presave(): When handling data, the golden rule is to store exactly what the user typed. When a user edits a post they created earlier, the form should contain the same things as it did when they first submitted it. This means that conversions are performed when content is output, not when saved to the database. See http://drupal.org/node/28984 . So I guess you can remove this function as a whole.
  14. "'#title' => onki_get_description($term->name),": that also looks suspicious, make sure that the description has been sanitized somewhere.
  15. "'fi' => 'Finnish',": all user facing text should run through t(). Please check all your #options values.
  16. throbber.gif: is that the standard drupal throbber? I think you can remove that file.