[ZBX-3104] Clock element with 'Host time' mode for templated screens. And other errors in screen. Created: 2010 Oct 16 Updated: 2017 May 30 Resolved: 2011 Nov 24 |
|
Status: | Closed |
Project: | ZABBIX BUGS AND ISSUES |
Component/s: | Frontend (F) |
Affects Version/s: | 1.9.0 (alpha) |
Fix Version/s: | 1.8.10, 1.9.9 (beta) |
Type: | Incident report | Priority: | Blocker |
Reporter: | Oleksii Zagorskyi | Assignee: | Unassigned |
Resolution: | Fixed | Votes: | 1 |
Labels: | screens, templates | ||
Remaining Estimate: | Not Specified | ||
Time Spent: | Not Specified | ||
Original Estimate: | Not Specified | ||
Environment: |
from trunk rev 14910. [DEV-483] added templated screens. |
Description |
Generally this is very nice feature, thanks. (4) For not templated screens need to check user select this parameters or not before saving. Try to save without selected host now. (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): heh |
Comments |
Comment by Oleksii Zagorskyi [ 2010 Oct 16 ] |
Correcting: |
Comment by Oleksii Zagorskyi [ 2010 Oct 16 ] |
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. <zalex> answer in the |
Comment by Oleksii Zagorskyi [ 2010 Oct 26 ] |
some parts from this is issue request now is resolved in |
Comment by Pavels Jelisejevs (Inactive) [ 2011 Nov 17 ] |
(1) "I think for templated screens this Parameter should be hided. " <zalex> Hmm, how the Clock element is able to display some data? As you see any templated screen resources (Graph, Plain text, Simple graph) are limited to select "Parameter" only from the same template. <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 |
Comment by Pavels Jelisejevs (Inactive) [ 2011 Nov 17 ] |
(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". " <zalex> seems is not reproducible in the Zabbix 1.9.8 r23287 <richlv> this was fixed as CLOSED |
Comment by Oleksii Zagorskyi [ 2011 Nov 17 ] |
added numbering to description |
Comment by Oleksii Zagorskyi [ 2011 Nov 17 ] |
(6) and (7) seems are not reproducible in the Zabbix 1.9.8 r23287 |
Comment by Pavels Jelisejevs (Inactive) [ 2011 Nov 21 ] |
(3, 4). I've added validation rules for all screen elements with the "parameter" field. It's now mandatory. RESOLVED. <asaveljevs> CLOSED |
Comment by Pavels Jelisejevs (Inactive) [ 2011 Nov 21 ] |
(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 |
Comment by Aleksandrs Saveljevs [ 2011 Nov 22 ] |
(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 |
Comment by Aleksandrs Saveljevs [ 2011 Nov 22 ] |
(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 |
Comment by Aleksandrs Saveljevs [ 2011 Nov 22 ] |
(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 |
Comment by Aleksandrs Saveljevs [ 2011 Nov 22 ] |
(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 |
Comment by Aleksandrs Saveljevs [ 2011 Nov 22 ] |
(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 |
Comment by Aleksandrs Saveljevs [ 2011 Nov 22 ] |
(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 <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 |
Comment by Aleksandrs Saveljevs [ 2011 Nov 23 ] |
(14) Function checkInput() in class ScreenItem does not validate host group for "Data overview" element. <pavels> RESOLVED. <asaveljevs> CLOSED |
Comment by Aleksandrs Saveljevs [ 2011 Nov 23 ] |
(15) Please review my changes regarding formatting and typos in r23453. <pavels> CLOSED. |
Comment by Pavels Jelisejevs (Inactive) [ 2011 Nov 24 ] |
Merged to trunk revision r23493. |
Comment by Pavels Jelisejevs (Inactive) [ 2011 Nov 24 ] |
I've backported the fix for 1.8. It's available at /branches/dev/ |
Comment by Pavels Jelisejevs (Inactive) [ 2011 Dec 01 ] |
Merged to trunk r23677 and 1.8 r23675. CLOSED. |
Comment by richlv [ 2012 Jan 27 ] |
turns out this resulted in a regression for "triggers info" screen element validation, filed as |