[D7] Taxonomy unique

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

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.

Still getting following errors
http://pareview.sh/pareview/httpgitdrupalorgsandboxsch4lly2135763git

FILE: /var/www/drupal-7-pareview/pareview_temp/taxonomy_unique.module
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
--------------------------------------------------------------------------------
71 | WARNING | Only string literals should be passed to t() where possible
--------------------------------------------------------------------------------

Also you should not use "master" branch in git. You should create a version specific branch.

You should delete the master branch.

Module functionality is working as expected
Try reducing number of lines of code especially in this function

function taxonomy_unique_is_term_unique($term_name, $vocabulary_machine_name, $tid = NULL) {

  $terms = taxonomy_get_term_by_name($term_name, $vocabulary_machine_name);

  // If taxonomy_get_term_by_name returns an empty array, name must be unique.
  if (empty($terms)) {
    return TRUE;
  }

  // If we insert a new term and taxonomy_get_term_by_name
  // found existing terms, name can't be unique.
  if ($tid == NULL) {
    return FALSE;
  }

  // If we update an existing term but taxonomy_get_term_by_name
  // found more than 1 terms, name can't be unique.
  if (count($terms) > 1) {
    return FALSE;
  }

  // If only one term is found and it shares the term ID
  // with the one we're trying to update, name must be unique.
  $term = current($terms);
  if ($term->tid == $tid) {
    return TRUE;
  }

  // No other condition matched, name can't be unique.
  return FALSE;
}

You should try like this

  if ($tid == NULL || count($terms) > 1) {
    return FALSE;
  }

Hey, good idea for a module and seems to be a unique option. Couple of remarks here:

1. I noticed that when I enabled the module, created duplicate terms and then enabled unique terms for that vocabulary I couldn't delete existing duplicate terms. Might be a scenario to check for.
2. Also, the project page could use some more details per https://drupal.org/node/997024, specifically screenshots (example: http://cl.ly/image/0S063I431S3O) and a link to documentation

Thanks!

Hi sch4lly,

Firstly your Git link in the summary should be:
git clone --branch 7.x http://git.drupal.org/sandbox/sch4lly/2135763.git taxonomy_unique

Your branch should be called "7.x-1.x" rather than "7.x" as you're not providing the version of the module; only the core requirement. This means you'd need to update the git path above again ;)

In "taxonomy_unique_is_term_unique()"

if ($tid == NULL) {
  return FALSE;
}

Would be better as

if ($tid === NULL) {
  return FALSE;
}

Nice module though, will definitely use this. I agree with neerajskydiver on the taxonomy_unique_is_term_unique() being too verbose.

Your git url is still wrong :)

Look at the one in my comment, you've got your username in yours.

One of the principles of the Drupal project is collaboration. Would you consider merging your project into that one and collaborating with the authors of Taxonomy Dupcheck.

Hello sch4lly ,

Great work , I just want to tell that ,
There are some coding standard problem and you can also reducing some number lines of code

<?php
function taxonomy_unique_is_term_unique($term_name, $vocabulary_machine_name, $tid = NULL) {

  $terms = taxonomy_get_term_by_name($term_name, $vocabulary_machine_name);

  if (empty($terms)) {
    return TRUE;
  }


  if ($tid === NULL || count($terms) > 1) {
    return FALSE;
  }

 
  $term = current($terms);
  if ($term->tid == $tid) {
    return TRUE;
  }


  return FALSE;
}

You should try like this

<?php
function taxonomy_unique_is_term_unique($term_name, $vocabulary_machine_name, $tid = NULL) {

  $terms = taxonomy_get_term_by_name($term_name, $vocabulary_machine_name);

  $term = current($terms);

  if (empty($terms) || $term->tid == $tid) {
    return TRUE;
  }

    return FALSE;
}

Hi sch4lly ,

occurences is misspelled in taxonomy unique.info. It should be occurrences.

Hi ch4lly,
I am unable to create a term with same name under parent term.

e.g. i want to create following category as followings:

Sahara
News
Z-tv
News

But i am unable to create same News category more than one time in different parent term.

Thanks sch4lly.

Hi Sch4lly,

No automated testing issues found, but one manual review issues:

1. JavaScript isn't being stripped / encoded from your error message. E.g. setting it to "Term %term already exists in vocabulary %vocabulary.
   alert(1);

   " displays the alert on error. You should wrap filter_xss() around the string when it's output. Don't trust anyone. Even the admin :)

It'd be great to have tests.

Add a minimal taxonomy tree using the API or DrupalWebTestCase::drupalPost(), and then try to post the same value as an existing term. Check that your validation error text appeared with assertText().

Check out the SimpleTest example if you need some guidance: https://drupal.org/project/examples

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.

There is an error on line 76
Parse error: syntax error, unexpected ';'

Looks like its missing a bracket for form_set_error .

Great work , I reviewd this module and it is working fine. Please add check_plain() for variables you used. This validates strings as UTF-8 to prevent cross site scripting attacks on Internet Explorer 6.

Example:
$term->tid and etc.

A term ID is not user provided text, so it does not need to be run through check_plain().

@sch4lly This link really helps.

Thanks.

Works as expected.

Review of the 7.x-1.x branch (commit 3bb3f5f):

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  FILE: /home/klausi/pareview_temp/taxonomy_unique.module
  -----------------------------------------------------------------------
  FOUND 5 ERRORS AFFECTING 5 LINES
  -----------------------------------------------------------------------
   84 | ERROR | Parameter comment must end with a full stop
   86 | ERROR | Parameter tags must be grouped together in a doc comment
   87 | ERROR | Parameter comment must end with a full stop
   89 | ERROR | Parameter tags must be grouped together in a doc comment
   90 | ERROR | Parameter comment must end with a full stop
  -----------------------------------------------------------------------
  

• No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

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. "'#title' => 'Terms should be unique.',": all user facing text must run through t() for translation. Also elsewhere, please check all your strings.
2. taxonomy_unique_taxonomy_form_vocabulary_submit(): doc block is wrong, this is a form submission handler. See https://www.drupal.org/coding-standards/docs#forms

But otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to er.pushoinderrana as he might have time to take a final look at this.

@sch4lly: The Git commits are not connected to your user account. You need to specify an email address. See https://www.drupal.org/node/1022156 and https://www.drupal.org/node/1051722

Ah, crosspost with @Daniel.Redwig, who also thinks this is RTBC. Which means we can approve this right away.

Thanks for your contribution, sch4lly!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.