The label field is already encoded,which is wrong, and the page title is encoded twice.

CommentFileSizeAuthor
#7 tmgmt-handling_labels-1975982-2.interdiff.txt1.54 KBblueminds
#7 tmgmt-handling_labels-1975982-2.patch5.59 KBblueminds
#3 tmgmt-handling_labels-1975982-1.patch5.61 KBblueminds

Comments

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

And while we're at it, the title should probably be shortened for both job and job item titles, looks crappy right with a long node title.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

What's worse, the label is limited to 128 characters but the auto-generated label can be bigger than that.

blueminds’s picture

blueminds commented
Issue summary: View changes
StatusFileSize
new tmgmt-handling_labels-1975982-1.patch5.61 KB

See the patch. However I am not sure if my approach where job defaultLabel() does not escape and the caller should escape is just, but made most sense to me.

Also did the label max length thing.

blueminds’s picture

blueminds commented
Status: Active » Needs review
miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

I guess, yes.
The inner API functions should return unencoded stuff. Encoding is a matter of the displaying layer.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented 
  1. +++ b/tests/tmgmt_test.plugin.source.inc
@@ -26,7 +26,16 @@ class TMGMTTestSourcePluginController extends TMGMTDefaultSourcePluginController
+    }
...
+    // We need to test if job and job item labels get properly truncated,
+    // therefore in case the job item type is "test_with_long_label" we append
+    // further text to the existing label.
+    if ($job_item->item_type == 'test_with_long_label') {
+      $label .= 'Some very long and boring label that definitely exceeds hundred and twenty eight characters or the TMGMT_JOB_LABEL_MAX_LENGTH constant value.';

    Why 128 or the constant? :) 

  2. +++ b/entity/tmgmt.entity.job.inc
@@ -127,8 +127,18 @@ class TMGMTJob extends Entity {
+      // If the label length exceeds maximum allowed then cut off exceeding
+      // characters from the title and use it to recreate the label.
+      if (TMGMT_JOB_LABEL_MAX_LENGTH > 0 && strlen($label) > TMGMT_JOB_LABEL_MAX_LENGTH) {

    The > 0 check seems a bit weird, it's a constant not a variable, so we don't really expect it to change...

blueminds’s picture

blueminds commented
StatusFileSize
new tmgmt-handling_labels-1975982-2.patch5.59 KB
new tmgmt-handling_labels-1975982-2.interdiff.txt1.54 KB

hmm, yes strange... it looks like the length would be going to be configurable... fixed

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

Thanks, committed and pushed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.