Ready for testing in development branch svn://svn.zabbix.com/branches/dev/ZBXNEXT-3572

(1) [F] Undefined variable: total [ in queue.php:302]
Frontend shows error message

vjaceslavs My bad. RESOLVED in r64956

vso CLOSED

(2) [S] New unnecessary goto label err: has been added.

Other places are satisfied with one out: label for example:

	if (FAIL == zbx_json_value_by_name(jp, ZBX_PROTO_TAG_SID, sessionid, sizeof(sessionid)) ||
		FAIL == zbx_session_validate(sessionid, USER_TYPE_SUPER_ADMIN))
	{
		zbx_send_response_raw(sock, ret, "Permission denied.", CONFIG_TIMEOUT);
		goto out;
	}

Also ret = SUCCEED Is set twice and even on error.

	ret = SUCCEED;
err:
	DCfree_item_queue(&queue);
	zbx_vector_ptr_destroy(&queue);

	zbx_json_free(&json);

	ret = SUCCEED;
out:

So I suggest changing the code to use only one out: which will help to avoid confusion.

vjaceslavs Other places are satisfied with out label because there is no memory allocation before those places. ret = SUCCEED removed from err as confusing code, but it does not change the logic of execution.

RESOLVED in r64958

vjaceslavs Moved check before memory allocation.

RESOLVED in r64959

vso REOPENED trapper.c:455:6: error: label ‘err’ used but not defined

vjaceslavs Sorry, missed something when merged code to svn.

RESOLVED in r64965

vso CLOSED with small improvement in r64967

(3) [S] Older frontend + new server show Total: 500 wen there is more than 500 items in queue.

Before that it would show Total: 500 (Truncated)

vjaceslavs Improved compatibility with old frontend versions, fixed specification. RESOLVED in r64970

vso CLOSED

(4) [F] There is no way to control amount of queued items through frontent interface, only by changing hardcoded parameter.

There are no changes planned in the UI of frontend to allow changing amount of queued items.

WON'T FIX
vso CLOSED

server side successfully tested.

TODO: frontend need to be tested/implemented.

(5) I think there is no need to leave the old solution for displaying the number of details, because it will work if users have updated the frontend only.

+		// fallback to old solution
+		$total = _('Total').': '.$table->getNumRows().
+			(count($queueData) > QUEUE_DETAIL_ITEM_COUNT ? ' ('._('Truncated').')' : '');

vjaceslavs WON'T FIX
For this solution frontend should be backward compatible with older server version.

(6) [F] No translation strings changed

vjaceslavs CLOSED

(7) [F]

1. useless empty div
2. this code is incorrectly formatted
3. empty footer shouldn't be displayed in overview mode

frontends/php/queue.php:298

->addItem((new CDiv())
        ->addClass(ZBX_STYLE_TABLE_PAGING)
        ->addItem((new CDiv())
            ->addClass(ZBX_STYLE_PAGING_BTN_CONTAINER)
            ->addItem(new CDiv())
                ->addItem((new CDiv())
                ->addClass(ZBX_STYLE_TABLE_STATS)
                ->addItem($total)
            )

sasha RESOLVED in r65246

vjaceslavs CLOSED

(8) [F] The following lines do not look to be according to coding guidelines:

frontends/php/include/classes/server/CZabbixServer.php:155

if ($type == self::QUEUE_DETAILS && isset($limit)) {

frontends/php/include/classes/server/CZabbixServer.php:273

$this->total = (isset($response['total'])) ? $response['total'] : null;

sasha RESOLVED in r65248

vjaceslavs CLOSED

Successfully tested! Have a look at my changes before a merge.

Available in:
3.0.8rc1 r65254
3.2.4rc1 r65256
pre-3.3.0 (trunk) r65257

(9) [AFS] new parameters limit and total should be mandatory in trunk.

vjaceslavs RESOLVED in r65386

sasha backward compatibility wasn't removed from CZabbixServer class

REOPENED

vjaceslavs Not sure if any of the changes made to CZabbixServer are related to backward compatibility. Changes are:

1. Introduced new class attribute total required in trunk
2. Introduced new attribute limit for method getQueue required in trunk
3. Introduced default value of attribute limit for method getQueue required in trunk
4. Introduced check for queue type to set value of limit method getQueue required in trunk
5. Introduced extraction of field total (if any) in method request required in trunk
6. Introduced getter for class attribute total required in trunk

All of those changes are required in trunk. The only change that is optional and that does not changing the logic is a small fix to reset state of server object in method request:

// reset object state
$this->error = null;
$this->total = null;

This is optional fix to allow execution of multiple consecutive requests (not really used right now, but makes sense as object state should not remain corrupted within multiple method calls).
Do you want to get rid of that?

sasha total parameter is mandatory for trunk. This code is really necessary?

$this->total = array_key_exists('total', $response) ? $response['total'] : null;

vjaceslavs total parameter is mandatory for only one response type (response to details of queue) in trunk. So we can't drop the check. Check can be changed so request type is checked, but it will make two different solutions for 3.2 and trunk.

sasha CLOSED

Available in:
3.0.8rc1 r65427
3.2.4rc1 r65428
pre-3.3.0 (trunk) r65430

(10) [D] I think it shall be added to what's new.
Also constants page shall be updated:
https://www.zabbix.com/documentation/3.2/manual/web_interface/definitions
Please see

// item count to display in the details queue
define('QUEUE_DETAIL_ITEM_COUNT', 500);

VSI
User documentation updated:

• Created "What's new" for versions 3.0.8 and 3.2.4 with corresponding items
• Description of QUEUE_DETAIL_ITEM_COUNT parameter added to Defenitions page for versions 3.0, 3.2 and 3.4

vso Actually it does not Defines limit of the total items queued. it defines how many queued items to retrieve at most.

VSI Rephrased into Defines retrieval limit of the total items queued.

vso Thanks, this is very good however in what's new it still says in the old style.

VSI Heading and description in "what's new" fixed.

vso What's new is not similar to definitions description.

QUEUE_DETAIL_ITEM_COUNT definition parameter now may be edited to retrieve more than default 500 queue requests.

But we don't retrieve any requests.

VSI Reworded with "500 values"

vso CLOSED

sasha Displaying of total count of queued items also must be mentioned. It is a new cool feature!

REOPENED

VSI More detailed description of visual changes added in What's new for 3.0.8 and 3.2.4.

sasha Perfect! CLOSED