To avoid such problems we've decided to remove the "Type" parameter from the image configuration form. Please make the following changes:

• Remove the "type" select from the image configuration form
• Change the title of the "Create image" button to "Create icon" and "Create background" depending on the selected type.
• Change the header of the image configuration form from "Image" to either "Icon" or "Background".
• Forbid updating the image "imagetype" parameter using the image.update method (see how it's done for item "state" and "templateid" in CItemGeneral::checkInput() line 203). Note, that it should still work in image.create.

hmm. in the past, accidentally uploading a background image as an icon could be corrected just by changing the type. with the suggested changes it would have to be deleted and re-uploaded.
it does sound like we will remove type selection from the upload, though - if so, that should mostly solve this problem.

as we are changing button label[s], i'd suggest to change them to "Upload icon" and "Upload background image" instead - having 'create' there is somewhat confusing

Eduards REOPEN Discussed with Sasha and Pavel, let's call button "Create icon" and "Create background"

kristsk RESOLVED in r44903. Chagned translatable strings:

• 'Upload icon' => 'Create icon'
• 'Upload background image' => 'Create background'

<richlv> in this context "create" is highly confusing and users have repeatedly asked why the button is labelled like that. please provide the reasoning to use "create" here.

Eduards CLOSED, "Create" is the name of business process, "upload" is method how we implement this functionality.

Resolved in r44746, r44747, branch svn://svn.zabbix.com/branches/dev/ZBX-8118.

(1) Translatable string changes:
New strings:

• 'Cannot update "imagetype" for image "%1$s".'
• 'Create background'
• 'Create icon'

Removed strings:

• 'Create image'
• 'Wrong fields for image.'

kristsk Updated translatable strings list. RESOLVED.

iivs CLOSED.

(2) adm.images.php:148 This code can be written in 1 line:

	if($imageType == IMAGE_TYPE_ICON) {
		$submitCaption = _('Upload icon');
	}
	else {
		$submitCaption = _('Upload background image');
	}

$submitCaption = ($imageType == IMAGE_TYPE_ICON) ? _('Upload icon') : _('Upload background image');

After this changes variable $submitCaption will be used once so also can be optimized..

kristsk RESOLVED in r44783.
Eduards CLOSED

(3) administration.general.image.edit.php:46
Incorrent ")" in line

$imageTab->addTab('imageTab', ($this->data['imagetype'] == IMAGE_TYPE_ICON ? _('Icon') : _('Background')), $imageFormList);

must be

$imageTab->addTab('imageTab', ($this->data['imagetype'] == IMAGE_TYPE_ICON) ? _('Icon') : _('Background'), $imageFormList);

kristsk RESOLVED in r44783.
Eduards CLOSED

(4) Incorrect string, please changes:
_('Can not update "%1$s" for image "%2$s".')
to
_('Cannot update "%1$s" for image "%2$s".')

Eduards CLOSED

(5) Undefined index in Image API request for query:
{"jsonrpc":"2.0","method":"image.update","params":

,"id":0,"auth":"303e0a760eabd231c43eaf916be1ee37"}

Notice: Undefined index: name in /home/zabbix/www/testing-ZBX-8118/frontends/php/api/classes/CImage.php on line 329

kristsk RESOLVED in r44793.

Eduards REOPEN, Empty name if we try to update unexisting image: "Cannot update \"imagetype\" for image \"\"."

{"jsonrpc":"2.0","method":"image.update","params":

,"id":0,"auth":"303e0a760eabd231c43eaf916be1ee37"}

Also:

• in this case will be faster to use API::getApiService()->select() to get data from DB. For example:

  $imageName = API::getApiService()->select($this->tableName(), array(
      'filter' => array('imageid' => $image['imageid']),
      'output' => array('name')
  ));
  

• Please use space between "if" and "(" in CImage.php:344

kristsk RESOLVED in r44900.
Eduards REOPEN as discussed
kristsk RESOLVED in r44944.

Eduards REOPEN little formatting improvements:

• line 509 require space between "foreach" and "("
• line 535 !empty() can be removed, just "if ($imageNames) {"
• line 544 redundant "()"

kristsk RESOLVED in r44968.
Eduards REOPEN

• line 522 require space between "if" and "("
• line 544 require tab at beginning of second line of if expression

Eduards CLOSED

(6) Please resolve conflicts after merging with trunk

kristsk RESOLVED in r45009.

Eduards CLOSED

Tested

Merged and committed to trunk in r45022.

(7) This needs to be documented in the API changelog.

kristsk RESOLVED.

jelisejev Great, thanks! CLOSED.

(8) Documented in general documentation as well:

• https://www.zabbix.com/documentation/2.4/manual/introduction/whatsnew240#changing_uploaded_image_type_disallowed
• https://www.zabbix.com/documentation/2.4/manual/web_interface/frontend_sections/administration/general#images

jelisejev Thanks! CLOSED.

subissues still open : 1