Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(Impact): use genericobject/front/getimpacticon.php to serve icon #368

Merged
merged 5 commits into from
Jan 8, 2024

Conversation

stonebuzz
Copy link
Contributor

Icon from genericobject is badly display from GLPI impact analysis

image

See relative path containing a double backslash

Clean relative icon path by removing extra backslash

as is already added by glpi core

https://github.com/glpi-project/glpi/blob/5d510f7cb9cd775a9477f98059a8e7569e2f904c/src/Impact.php#L1029

@stonebuzz stonebuzz added the bug label Dec 21, 2023
@stonebuzz stonebuzz self-assigned this Dec 21, 2023
inc/type.class.php Outdated Show resolved Hide resolved
@stonebuzz
Copy link
Contributor Author

After applying fix

image

Copy link
Contributor

@cedric-anne cedric-anne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

files directory is not supposed to be exposed. It may be the time to fix it.

@cedric-anne cedric-anne added this to the 2.14.9 milestone Dec 21, 2023
@stonebuzz
Copy link
Contributor Author

the first solution may not work if GLPI's "fiels" folder is in a different location (downstream.php and GLPI_VAR_DIR constant)

using pluginimage.send.php seems more appropriate

But does not allow to specify a subfolder

We have to wait for this PR

glpi-project/glpi#16241

@stonebuzz stonebuzz changed the title fix(Impact): remove backslash added twice fix(Impact): use pluginimage.send.php to serve icon Dec 21, 2023
Copy link
Contributor

@trasher trasher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You'll have to set minimal GLPI version to 10.0.12 if glpi-project/glpi#16241 is needed.

@cedric-anne cedric-anne removed this from the 2.14.9 milestone Dec 21, 2023
Copy link
Contributor

@AdrienClairembault AdrienClairembault left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't add an icon to my custom objects:

[2023-12-26 10:20:04] glpiphplog.WARNING:   *** PHP Warning (2): rename(/home/teclib/localhost/glpi/10.0-bugfixes/files/_tmp/60d67067881900.71152486B.png,front/pluginimage.send.php?plugin=genericobject&folder=impact_icons&name=PluginGenericobjectVoiture_60d67067881900.71152486B.png): No such file or directory in /home/teclib/localhost/glpi/10.0-bugfixes/plugins/genericobject/inc/type.class.php at line 289
  Backtrace :
  plugins/genericobject/inc/type.class.php:289       rename()
  plugins/genericobject/inc/type.class.php:218       PluginGenericobjectType->handleImpactIconUpdate()
  src/CommonDBTM.php:1612                            PluginGenericobjectType->prepareInputForUpdate()
  plugins/genericobject/front/type.form.php:51       CommonDBTM->update()

@AdrienClairembault
Copy link
Contributor

Also could you confirm the specific case where the original issue happens ? Icons work fine on my side so I suppose its only with a specific config ?

@stonebuzz
Copy link
Contributor Author

stonebuzz commented Jan 8, 2024

GLPI 10.0.12-dev
Genericobject -> master branch
GLPI router enabled
downstream.php disabled

Icon added to generic object
image

Icon displayed from impact analysis tab (from generic object)
image

Icon displayed from impact analysis tab (from computer object)
image

computed path :
image

GLPI 10.0.12-dev
Genericobject -> master branch
GLPI router enabled
downstream.php enabled

Default icon is displayed from impact analysis tab (from generic object)
image

Default icon is displayed from impact analysis tab (from computer object)
image

computed path :
image

I think the problem is with the GLPI router and/or downstream.

I've reworked the PR because the plugin already uses a dedicated front-end (front/getimpacticon.php) to load the image

(it's no longer necessary to use pluginimage.send.php).

@stonebuzz
Copy link
Contributor Author

stonebuzz commented Jan 8, 2024

It doesn't matter whether the image is displayed in the impact analysis or in the object configuration.

The plugin will always use getImpactIconUrl (dedicated front getimpacticon.php to not exposed files directory).

Only the call from the impact analysis does not have to contain the full path to prevent double backslash )

@stonebuzz stonebuzz changed the title fix(Impact): use pluginimage.send.php to serve icon fix(Impact): use genericobject/front/getimpacticon.php to serve icon Jan 8, 2024
@cedric-anne cedric-anne added this to the 2.14.9 milestone Jan 8, 2024
@cedric-anne cedric-anne merged commit b7f05dd into pluginsGLPI:master Jan 8, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants