ZABBIX BUGS AND ISSUES

There is possibility to create value mappings with the same name

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Blocker Blocker
  • Resolution: Fixed
  • Affects Version/s: 1.9.9 (beta)
  • Fix Version/s: 1.8.11, 1.9.9 (beta)
  • Component/s: Frontend (F)
  • Labels:
  • Environment:
    1.9.9 (trunk), 1.8.x
  • Zabbix ID:
    Reviewed 2.0

Description

There is possibility to create value mappings with the same name (field name in the Zabbix DB), the only difference between such mappings will be value of the "valuemapid" field.

In v1.8.x there is the same behaviour.

Issue Links

Activity

Hide
Pavels Jelisejevs added a comment - - edited

(1) Don't use concatenation in messages, use the _s() function instead.
<Slava>RESOLVED r 24281

<pavels> Not sprintf() but _s(). It's our standard gettext wrapper for strings with arguments.
<Slava> RESOLVED r 24506

<pavels> CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (1) Don't use concatenation in messages, use the _s() function instead. <Slava>RESOLVED r 24281 <pavels> Not sprintf() but _s(). It's our standard gettext wrapper for strings with arguments. <Slava> RESOLVED r 24506 <pavels> CLOSED.
Hide
Pavels Jelisejevs added a comment - - edited

(2) The input parameters that we use in SQL queries should always be escaped. Use the zbx_dbstr to escape the name in get_valuemap()
<SlaVa>RESOLVED r 24281

<pavels> CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (2) The input parameters that we use in SQL queries should always be escaped. Use the zbx_dbstr to escape the name in get_valuemap() <SlaVa>RESOLVED r 24281 <pavels> CLOSED.
Hide
Pavels Jelisejevs added a comment - - edited

(3) We use upperCamelCase for function names. Plus, I suggest we rename the get_valuemap function to getValuemapByName($name) and make the $name parameter obligatory.
<SlaVa>RESOLVED r 24281

<pavels> One more thing, all the tables in SQL queries must use an alias. Even if there's only one table.
<Slava> RESOLVED r 24506

<pavels> CLOSED.

Show
Pavels Jelisejevs added a comment - - edited (3) We use upperCamelCase for function names. Plus, I suggest we rename the get_valuemap function to getValuemapByName($name) and make the $name parameter obligatory. <SlaVa>RESOLVED r 24281 <pavels> One more thing, all the tables in SQL queries must use an alias. Even if there's only one table. <Slava> RESOLVED r 24506 <pavels> CLOSED.
Hide
Alexander Vladishev added a comment - - edited

(4) [GUI] Please review my changes in r24304
<Slava>CLOSED

Show
Alexander Vladishev added a comment - - edited (4) [GUI] Please review my changes in r24304 <Slava>CLOSED
Hide
Alexander Vladishev added a comment - - edited

(5) [GUI] Cannot update value map
ERROR: Cannot add value map. Map with name "Service state" already exist
<Slava>RESOLVED r 24359

<pavels> Still not working. REOPENED.
<Slava> RESOLVED r24490

<pavels> I thing you've meant
isset($_REQUEST['valuemapid']) && bccomp($_REQUEST['valuemapid'], $prevMap['valuemapid']) == 0
intead of
isset($prevMap['valuemapid']) && bccomp($_REQUEST['valuemapid'], $prevMap['valuemapid']) == 0
<Slava>Resolved r 24498

<pavels> CLOSED.

Show
Alexander Vladishev added a comment - - edited (5) [GUI] Cannot update value map ERROR: Cannot add value map. Map with name "Service state" already exist <Slava>RESOLVED r 24359 <pavels> Still not working. REOPENED. <Slava> RESOLVED r24490 <pavels> I thing you've meant isset($_REQUEST['valuemapid']) && bccomp($_REQUEST['valuemapid'], $prevMap['valuemapid']) == 0 intead of isset($prevMap['valuemapid']) && bccomp($_REQUEST['valuemapid'], $prevMap['valuemapid']) == 0 <Slava>Resolved r 24498 <pavels> CLOSED.
Hide
richlv added a comment - - edited

(6) it should be "already exists"
<Slava>fixed
it should be also verified whether the message should have the trailing dot or not (according to the guidelines)
<SlaVa>https://zabbix.org/wiki/Docs/specs/syntax :
2 . Main status messages like "Host added" (displayed on green/red background at the top of the page) should never end with a dot.
RESOLVED

<pavels> CLOSED.

Show
richlv added a comment - - edited (6) it should be "already exists" <Slava>fixed it should be also verified whether the message should have the trailing dot or not (according to the guidelines) <SlaVa>https://zabbix.org/wiki/Docs/specs/syntax : 2 . Main status messages like "Host added" (displayed on green/red background at the top of the page) should never end with a dot. RESOLVED <pavels> CLOSED.
Hide
Igor Danoshaites added a comment - - edited

(5) [GUI] "Cannot add or update value map" -->For this on 30.12.2011 has been created separate issue ZBX-4498

<pavels> This fix hasn't been merged yet.
<Slava> fixed in r 24515

Show
Igor Danoshaites added a comment - - edited (5) [GUI] "Cannot add or update value map" -->For this on 30.12.2011 has been created separate issue ZBX-4498 <pavels> This fix hasn't been merged yet. <Slava> fixed in r 24515
Hide
Pavels Jelisejevs added a comment - - edited

(6) Please review my changes in r24512.
<Slava>CLOSED

Show
Pavels Jelisejevs added a comment - - edited (6) Please review my changes in r24512. <Slava>CLOSED
Hide
Pavels Jelisejevs added a comment -

TESTED.

Please close (6) before merging. Also, after updating to the latest trunk, please, fix (5).

Show
Pavels Jelisejevs added a comment - TESTED. Please close (6) before merging. Also, after updating to the latest trunk, please, fix (5).
Hide
Vjacheslav Shipillo added a comment -

<Slava> fixed in r 24515

Show
Vjacheslav Shipillo added a comment - <Slava> fixed in r 24515
Hide
richlv added a comment - - edited

(7) merge to trunk has lots of coding guideline violations and modifies several unrelated changelog entries. i would strongly suggest reverting it, reviewing all modified code for coding style and doublechecking what it does to the changelog

edit : ok, changelog probably was intentional. why wasn't it mentioned in the commit message ?
<Slava> Sorry. I must recommit this with detailed description ?

<richlv> well, this time i'd say that it would be enough to fix formatting for all modified lines - but in future, describing all changes in the commit message would be highly appreciated
<Slava> You are so kind. RESOLVED r 24542

<richlv> 24542 has a commit message "updated to latest trunk "...
<Slava>Formating error starts after conflict resolving in 'valuemap.inc.php' . Now i re-resolve this conflicts.

<pavels> CLOSED.

Show
richlv added a comment - - edited (7) merge to trunk has lots of coding guideline violations and modifies several unrelated changelog entries. i would strongly suggest reverting it, reviewing all modified code for coding style and doublechecking what it does to the changelog edit : ok, changelog probably was intentional. why wasn't it mentioned in the commit message ? <Slava> Sorry. I must recommit this with detailed description ? <richlv> well, this time i'd say that it would be enough to fix formatting for all modified lines - but in future, describing all changes in the commit message would be highly appreciated <Slava> You are so kind. RESOLVED r 24542 <richlv> 24542 has a commit message "updated to latest trunk "... <Slava>Formating error starts after conflict resolving in 'valuemap.inc.php' . Now i re-resolve this conflicts. <pavels> CLOSED.
Hide
Vjacheslav Shipillo added a comment - - edited

<Slava> Pavel, please review my confict resolving in 24542 'valuemap.inc.php'

<pavels> CLOSED. Before merging to trunk, please review my changes in r24578 and r24579.
<Slava> CLOSED

Show
Vjacheslav Shipillo added a comment - - edited <Slava> Pavel, please review my confict resolving in 24542 'valuemap.inc.php' <pavels> CLOSED. Before merging to trunk, please review my changes in r24578 and r24579. <Slava> CLOSED
Hide
Vjacheslav Shipillo added a comment -

fixed in trunk in 24575

Show
Vjacheslav Shipillo added a comment - fixed in trunk in 24575
Hide
Pavels Jelisejevs added a comment - - edited

Fix for 1.8 is TESTED.

Please review my changes before merging.
<Slava> CLOSED

Show
Pavels Jelisejevs added a comment - - edited Fix for 1.8 is TESTED. Please review my changes before merging. <Slava> CLOSED
Hide
Vjacheslav Shipillo added a comment -

fixed in 1.8 r 24593

Show
Vjacheslav Shipillo added a comment - fixed in 1.8 r 24593
Hide
Oleksiy Zagorskyi added a comment -

Reopened to replace 2.0 to 1.9.9 in the Fix version/s

Show
Oleksiy Zagorskyi added a comment - Reopened to replace 2.0 to 1.9.9 in the Fix version/s
Hide
Oleksiy Zagorskyi added a comment -

Closed again.

Show
Oleksiy Zagorskyi added a comment - Closed again.

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: