Skip to content

Conversation

@Hipska
Copy link
Collaborator

@Hipska Hipska commented Sep 4, 2025

Base information

Question Answer
Related to a SourceForge thread / Another PR / Combodo ticket? N/A
Type of change? Enhancement

Objective (enhancement)

Helper method to easily get installed version of a module in iTop.

Proposed solution (bug and enhancement)

Added GetModuleVersion method to Utils class. In the meantime consecutive calls to the CheckModuleInstallation method for the same module will be faster as the result is now cached.

Checklist before requesting a review

  • I have performed a self-review of my code, and that it's compliant with Combodo's guidelines
  • I have tested all changes I made on an iTop instance
  • I have added a unit test, otherwise I have explained why I couldn't
  • I have made sure the PR is clear and detailled enough so anyone can understand the real purpose without digging in the code

@jf-cbd jf-cbd changed the title feat(Utils): Add method to retrieve module version N°8892 - feat(Utils): Add method to retrieve module version Nov 6, 2025
@jf-cbd jf-cbd requested a review from odain-cbd November 7, 2025 09:51
public static function GetModuleVersion(string $sModuleId, RestClient $oClient = null): string
{
if (!isset(static::$aModuleVersions[$sModuleId])) static::CheckModuleInstallation($sModuleId, true, $oClient);
return static::$aModuleVersions[$sModuleId];
Copy link
Contributor

Choose a reason for hiding this comment

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

what if key i s still missing?

Copy link
Contributor

Choose a reason for hiding this comment

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

could you also add tests please?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, but first wanted to know if Combodo would accept this addition.

If the module is not found, an exception will be thrown, so this return statement will not be reached..

Copy link
Contributor

Choose a reason for hiding this comment

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

current PR was accepted functionnally. I am here to check technical details before merging it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: First review needed

Development

Successfully merging this pull request may close these issues.

2 participants