ZABBIX BUGS AND ISSUES
  1. ZABBIX BUGS AND ISSUES
  2. ZBX-3104

Clock element with 'Host time' mode for templated screens. And other errors in screen.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 1.9.0 (alpha)
    • Fix Version/s: 1.8.10, 1.9.9 (beta)
    • Component/s: Frontend (F)
    • Labels:
    • Environment:
      from trunk rev 14910. [DEV-483] added templated screens.
      my DB schema always correctly updated to corresponding revision.

      Description

      Generally this is very nice feature, thanks.
      (3) But try set 'Host time' mode for Clock element for templated screens without selecting Parameter field and save. In bottom of page see error in editor and also in view mode (Monitoring -> Hosts -> Screens).
      (1) I think for templated screens this Parameter should be hided.

      (4) For not templated screens need to check user select this parameters or not before saving. Try to save without selected host now.
      (5) And in pop-up window need filter to real hosts only without templates.

      (6) And second serious new problem - now after save element is not updated on screen immediately. It need to click something and return back or refresh page to see changes. The same behavior for column, line add/remove.

      (7) In Monitoring -> Hosts need to add single space symbol between Hostname and DNS-name.

      (2) And not related only to trunk rev 14910 (something early revision):
      In not templated screens "Hostname:" is not presented in Parameter field if you open elements (Simple graph, clock and maybe other). It presented for element Graph and maybe other (but only after save and open element settings again). Try to choose Graph, Simple graph then save and open again and you will understand me.
      If you open pop-up window for host selection and try to change host group then you receive error "Warning. Field [submitParent] is not integer".

      heh

        Activity

        Hide
        Oleksiy Zagorskyi added a comment -

        Correcting:
        In Monitoring -> Hosts need to add single space symbol between IP and DNS-name.

        Show
        Oleksiy Zagorskyi added a comment - Correcting: In Monitoring -> Hosts need to add single space symbol between IP and DNS-name.
        Hide
        Oleksiy Zagorskyi added a comment - - edited

        Templated screen added by "+" button is not presented on Dashboard in 'Favourite screens' widget. And templated screen cannot be added in this widget by using pop-up menu.
        And may be templated screen should be available directly in Monitoring -> Screens with filter like an templated graph?

        <zalex> answer in the ZBX-3129
        CLOSED

        Show
        Oleksiy Zagorskyi added a comment - - edited Templated screen added by "+" button is not presented on Dashboard in 'Favourite screens' widget. And templated screen cannot be added in this widget by using pop-up menu. And may be templated screen should be available directly in Monitoring -> Screens with filter like an templated graph? <zalex> answer in the ZBX-3129 CLOSED
        Hide
        Oleksiy Zagorskyi added a comment - - edited

        some parts from this is issue request now is resolved in ZBX-3129
        and error about "Warning. Field [submitParent] is not integer" (last my sentence in this is issue Description) is not reproducible now.

        Show
        Oleksiy Zagorskyi added a comment - - edited some parts from this is issue request now is resolved in ZBX-3129 and error about "Warning. Field [submitParent] is not integer" (last my sentence in this is issue Description) is not reproducible now.
        Hide
        Pavels Jelisejevs added a comment - - edited

        (1) "I think for templated screens this Parameter should be hided. "
        Why? What if I want to create a screen item that displays some data from an item in the same template?

        <zalex> Hmm, how the Clock element is able to display some data?
        Hard to recall currently what I meant when I created this issue. I guess to hide from the GUI all related to "select "Parameter"".

        As you see any templated screen resources (Graph, Plain text, Simple graph) are limited to select "Parameter" only from the same template.
        Why the Clock resource doesn't limited in the same way?
        I don't have strong opinion in this case. Seems currently I even don't know what "Host time" mode should display

        <pavels> The host time displays the local time on the monitored host. It receives data from some special item (can't recall the key), that's why it needs the parameter field. Guess that makes (1) CLOSED.

        Regarding the limited choice of items, I posted a separate commend below.

        <richlv> system.localtime, i guess

        <asaveljevs> Item key is "system.localtime[local]", to be more precise.

        <asaveljevs> CLOSED

        Show
        Pavels Jelisejevs added a comment - - edited (1) "I think for templated screens this Parameter should be hided. " Why? What if I want to create a screen item that displays some data from an item in the same template? <zalex> Hmm, how the Clock element is able to display some data? Hard to recall currently what I meant when I created this issue . I guess to hide from the GUI all related to "select "Parameter"". As you see any templated screen resources (Graph, Plain text, Simple graph) are limited to select "Parameter" only from the same template. Why the Clock resource doesn't limited in the same way? I don't have strong opinion in this case. Seems currently I even don't know what "Host time" mode should display <pavels> The host time displays the local time on the monitored host. It receives data from some special item (can't recall the key), that's why it needs the parameter field. Guess that makes (1) CLOSED. Regarding the limited choice of items, I posted a separate commend below. <richlv> system.localtime, i guess <asaveljevs> Item key is "system.localtime [local] ", to be more precise. <asaveljevs> CLOSED
        Hide
        Pavels Jelisejevs added a comment - - edited

        (2) "If you open pop-up window for host selection and try to change host group then you receive error "Warning. Field [submitParent] is not integer". "
        I'm guessing it's already fixed?

        <zalex> seems is not reproducible in the Zabbix 1.9.8 r23287

        <richlv> this was fixed as ZBX-3042; also, as can be seen in zalex's comment above, dated 2010 Oct 26, he noted that this is not reproducible anymore

        CLOSED

        Show
        Pavels Jelisejevs added a comment - - edited (2) "If you open pop-up window for host selection and try to change host group then you receive error "Warning. Field [submitParent] is not integer". " I'm guessing it's already fixed? <zalex> seems is not reproducible in the Zabbix 1.9.8 r23287 <richlv> this was fixed as ZBX-3042 ; also, as can be seen in zalex's comment above, dated 2010 Oct 26, he noted that this is not reproducible anymore CLOSED
        Hide
        Oleksiy Zagorskyi added a comment -

        added numbering to description

        Show
        Oleksiy Zagorskyi added a comment - added numbering to description
        Hide
        Oleksiy Zagorskyi added a comment -

        (6) and (7) seems are not reproducible in the Zabbix 1.9.8 r23287
        CLOSED

        Show
        Oleksiy Zagorskyi added a comment - (6) and (7) seems are not reproducible in the Zabbix 1.9.8 r23287 CLOSED
        Hide
        Pavels Jelisejevs added a comment - - edited

        (3, 4). I've added validation rules for all screen elements with the "parameter" field. It's now mandatory. RESOLVED.

        <asaveljevs> CLOSED

        Show
        Pavels Jelisejevs added a comment - - edited (3, 4). I've added validation rules for all screen elements with the "parameter" field. It's now mandatory. RESOLVED. <asaveljevs> CLOSED
        Hide
        Pavels Jelisejevs added a comment - - edited

        (5) RESOLVED. Plus I've made, that the selection of items for a templated clock should be limited to the items from the template.

        <asaveljevs> The popup for selecting an item for clock element shows template's technical name in the upper right corner. It should show the visible name instead.

        <pavels> RESOLVED

        <asaveljevs> CLOSED

        Show
        Pavels Jelisejevs added a comment - - edited (5) RESOLVED. Plus I've made, that the selection of items for a templated clock should be limited to the items from the template. <asaveljevs> The popup for selecting an item for clock element shows template's technical name in the upper right corner. It should show the visible name instead. <pavels> RESOLVED <asaveljevs> CLOSED
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (8) File include/screens.inc.php, line 603 puts technical host name into $caption. It should put visible host name instead.

        <pavels> RESOLVED

        <asaveljevs> OK, but after the item is chosen and the popup is closed, the "Parameter" field in "Screen cell configuration" contains host's technical name.

        <pavels> Right. Fixed. RESOLVED.

        <asaveljevs> CLOSED

        Show
        Aleksandrs Saveljevs added a comment - - edited (8) File include/screens.inc.php, line 603 puts technical host name into $caption. It should put visible host name instead. <pavels> RESOLVED <asaveljevs> OK, but after the item is chosen and the popup is closed, the "Parameter" field in "Screen cell configuration" contains host's technical name. <pavels> Right. Fixed. RESOLVED. <asaveljevs> CLOSED
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (9) Clock element links to zabbix.com. It probably shouldn't.

        <asaveljevs> If clock element with "Host time" is present in a templated screen, then on the host level it will try to show data from the item in the template. It should show data from host's item.

        <asaveljevs> Also, the clock element shows technical host name. It should show visible host name.

        <pavels> i've created a separate issue for these problems ZBX-4378. CLOSED for now.

        Show
        Aleksandrs Saveljevs added a comment - - edited (9) Clock element links to zabbix.com. It probably shouldn't. <asaveljevs> If clock element with "Host time" is present in a templated screen, then on the host level it will try to show data from the item in the template. It should show data from host's item. <asaveljevs> Also, the clock element shows technical host name. It should show visible host name. <pavels> i've created a separate issue for these problems ZBX-4378 . CLOSED for now.
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (10) When selecting items for clock and plain text elements, item key column is not shown in the popup.

        <pavels> It's never shown when the popup displays items from a single host. Should it be?

        <asaveljevs> I think it should. As a server side developer, I don't care how my items are described - I do care about their keys. So when I search the popup, I search for the key, not description.

        <pavels> My mistake, the "key" column is displayed for the clock popup, it was only missing in the plain text popup. Fixed it. RESOLVED.

        <asaveljevs> CLOSED

        Show
        Aleksandrs Saveljevs added a comment - - edited (10) When selecting items for clock and plain text elements, item key column is not shown in the popup. <pavels> It's never shown when the popup displays items from a single host. Should it be? <asaveljevs> I think it should. As a server side developer, I don't care how my items are described - I do care about their keys. So when I search the popup, I search for the key, not description. <pavels> My mistake, the "key" column is displayed for the clock popup, it was only missing in the plain text popup. Fixed it. RESOLVED. <asaveljevs> CLOSED
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (11) New file frontends/php/api/classes/class.cscreenitem.php does not have any properties set. It should have svn:eol-style property set to native.

        <pavels> RESOLVED

        <asaveljevs> CLOSED

        Show
        Aleksandrs Saveljevs added a comment - - edited (11) New file frontends/php/api/classes/class.cscreenitem.php does not have any properties set. It should have svn:eol-style property set to native. <pavels> RESOLVED <asaveljevs> CLOSED
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (12) That new file has text indented with tabs in PHP doc comments. Is that our new convention?

        <pavels> I think PHP doc wasn't mentioned in our coding style guidelines, but IMHO it's more readable that way.

        <richlv> if something is missing in the conventions/guidelines, it must be added (preferably at https://zabbix.org/wiki/Docs/specs/coding_style)

        <pavels> So which style do we choose? With aligned parameter description, or without?

        <pavels> We discussed it yesterday and settled for aligned style. Added it to the docs https://zabbix.org/wiki/Docs/specs/coding_style#PHPDoc. RESOLVED.

        <asaveljevs> Nice! CLOSED

        Show
        Aleksandrs Saveljevs added a comment - - edited (12) That new file has text indented with tabs in PHP doc comments. Is that our new convention? <pavels> I think PHP doc wasn't mentioned in our coding style guidelines, but IMHO it's more readable that way. <richlv> if something is missing in the conventions/guidelines, it must be added (preferably at https://zabbix.org/wiki/Docs/specs/coding_style ) <pavels> So which style do we choose? With aligned parameter description, or without? <pavels> We discussed it yesterday and settled for aligned style. Added it to the docs https://zabbix.org/wiki/Docs/specs/coding_style#PHPDoc . RESOLVED. <asaveljevs> Nice! CLOSED
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (13) Class Screen uses "if(!is_null($options['countOutput']))" in method get(). However, class ScreenItem uses "if ($options['countOutput'])" directly. Is that OK?

        <asaveljevs> Similarly, starting from line 424 and below the code uses different style for checking empty arrays: "if(!empty($hostgroups))" and "if ($hosts)".

        <pavels> "is_null()" is not exactly the same as just "if()", but it seems weird, that if we pass, for instance, "countOutput = fallse", "!is_null" will return true, and the "countOption" will be executed.

        "if(!empty($hostgroups))" and "if ($hosts)" is the same thing, as long as the $hosts variable is defined.

        <asaveljevs> Oh, sure, but it would be nice to use the same style everywhere.

        <pavels> I've replaced the API options checks for "is not null" to be consistent with other APIs, but I really see no reason to use the empty() and is_null() functions.

        <richlv> ...and we should document in the guidelines which ones should be used

        <asaveljevs> You have chosen to use "$var === null" style instead of "is_null($var)" style used elsewhere. Why?

        $ grepphp options | grep null | grep -c is.null
        1025
        grepphp options | grep null | grep -vc is.null
        24

        <pavels> I, pesonally, think, that the === syntax is more readable, then is_null. E.g. if you check the return value of a function, foo() === null is much better then is_null(foo()) Plus, it's a bit faster then is_null: it doesn't have the overhead of calling a function.

        <pavels> Added it to the style guidelines https://zabbix.org/wiki/Docs/specs/coding_style#PHP_conventions RESOLVED.

        <asaveljevs> CLOSED

        Show
        Aleksandrs Saveljevs added a comment - - edited (13) Class Screen uses "if(!is_null($options ['countOutput'] ))" in method get(). However, class ScreenItem uses "if ($options ['countOutput'] )" directly. Is that OK? <asaveljevs> Similarly, starting from line 424 and below the code uses different style for checking empty arrays: "if(!empty($hostgroups))" and "if ($hosts)". <pavels> "is_null()" is not exactly the same as just "if()", but it seems weird, that if we pass, for instance, "countOutput = fallse", "!is_null" will return true, and the "countOption" will be executed. "if(!empty($hostgroups))" and "if ($hosts)" is the same thing, as long as the $hosts variable is defined. <asaveljevs> Oh, sure, but it would be nice to use the same style everywhere. <pavels> I've replaced the API options checks for "is not null" to be consistent with other APIs, but I really see no reason to use the empty() and is_null() functions. <richlv> ...and we should document in the guidelines which ones should be used <asaveljevs> You have chosen to use "$var === null" style instead of "is_null($var)" style used elsewhere. Why? $ grepphp options | grep null | grep -c is.null 1025 grepphp options | grep null | grep -vc is.null 24 <pavels> I, pesonally, think, that the === syntax is more readable, then is_null. E.g. if you check the return value of a function, foo() === null is much better then is_null(foo()) Plus, it's a bit faster then is_null: it doesn't have the overhead of calling a function. <pavels> Added it to the style guidelines https://zabbix.org/wiki/Docs/specs/coding_style#PHP_conventions RESOLVED. <asaveljevs> CLOSED
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (14) Function checkInput() in class ScreenItem does not validate host group for "Data overview" element.

        <pavels> RESOLVED.

        <asaveljevs> CLOSED

        Show
        Aleksandrs Saveljevs added a comment - - edited (14) Function checkInput() in class ScreenItem does not validate host group for "Data overview" element. <pavels> RESOLVED. <asaveljevs> CLOSED
        Hide
        Aleksandrs Saveljevs added a comment - - edited

        (15) Please review my changes regarding formatting and typos in r23453.

        <pavels> CLOSED.

        Show
        Aleksandrs Saveljevs added a comment - - edited (15) Please review my changes regarding formatting and typos in r23453. <pavels> CLOSED.
        Hide
        Pavels Jelisejevs added a comment -

        Merged to trunk revision r23493.

        Show
        Pavels Jelisejevs added a comment - Merged to trunk revision r23493.
        Hide
        Pavels Jelisejevs added a comment -

        I've backported the fix for 1.8. It's available at /branches/dev/ZBX-3104-18. Please review.

        Show
        Pavels Jelisejevs added a comment - I've backported the fix for 1.8. It's available at /branches/dev/ ZBX-3104 -18. Please review.
        Hide
        Pavels Jelisejevs added a comment -

        Merged to trunk r23677 and 1.8 r23675.

        CLOSED.

        Show
        Pavels Jelisejevs added a comment - Merged to trunk r23677 and 1.8 r23675. CLOSED.
        Hide
        richlv added a comment -

        turns out this resulted in a regression for "triggers info" screen element validation, filed as ZBX-4593

        Show
        richlv added a comment - turns out this resulted in a regression for "triggers info" screen element validation, filed as ZBX-4593

          People

          • Assignee:
            Pavels Jelisejevs
            Reporter:
            Oleksiy Zagorskyi
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: