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.

Sign up for GitHub

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

Jump to bottom

N°7644 - Add Brand logo and Model picture #646

Merged
merged 3 commits into from
Jul 10, 2024
Merged

N°7644 - Add Brand logo and Model picture #646

merged 3 commits into from
Jul 10, 2024

Conversation

Hipska
Copy link
Contributor

@Hipska Hipska commented May 2, 2024 •
edited
Loading

Base information

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

Objective

Some people (including me) like it to also document the brand logo of their devices. It would also be nice to have a picture of a specific model for easier recognition.

Proposed solution

  • Added a new logo AttributeImage field to the class Brand and use that also as image_attribute:

Screenshot of a Brand object having the logo

Screenshot of a Server creation form selecting the Brand

  • Added a new picture AttributeImage field to the class Model and use that also as image_attribute:

Screenshot of a Model object having a picture and mouseover on the brand link

  • Also created a new summary card for Brand to be able to see the logo when hovering on a brand link, as seen in previous screenshot.

I don't think it's relevant to add unit tests to the datamodel classes and fields.

Checklist before requesting a review

  • I have performed a self-review of my code
  • 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
  • Is the PR clear and detailed enough so anyone can understand digging in the code?

@Molkobain Molkobain self-assigned this May 2, 2024
@Molkobain Molkobain added the enhancement New feature or request label May 2, 2024
@Molkobain
Copy link
Contributor

Thanks for the detailed PR Thomas!
Ok for me.

I'll just need to remind to add an audit check in the Designer / migration notes for this new XML nodes.

@Hipska Hipska mentioned this pull request May 3, 2024
Merged
@Hipska
Copy link
Contributor Author

Hipska commented May 6, 2024

I'm starting to think it doesn't add much to have the picture for Model as its image_attribute, but maybe it would be nicer to have the picture on the summary card?

@Molkobain
Copy link
Contributor

I'm starting to think it doesn't add much to have the picture for Model as its image_attribute, but maybe it would be nicer to have the picture on the summary card?

On the summary card as well yes, it could be great :)

@Hipska
Copy link
Contributor Author

Hipska commented May 6, 2024

Instead of as image_attribute? Or both?

@Molkobain
Copy link
Contributor

Both I think, that way the semantic stays and can be used elsewhere.

@Hipska
Copy link
Contributor Author

Hipska commented May 6, 2024

Done, I'm only not sure if the type should stay in the summary card.

@Molkobain
Copy link
Contributor

Leave it as is, product team will decide whether they want to keep it or not :)

@Molkobain
Copy link
Contributor

Functional review: Accepted for iTop 3.2

@Molkobain
Copy link
Contributor

Done, I'm only not sure if the type should stay in the summary card.

POs rather keep the type on the summary card

@Molkobain Molkobain added this to the 3.2.0 milestone Jul 10, 2024
@Molkobain Molkobain changed the base branch from develop to support/3.2 July 10, 2024 07:38
@Molkobain
Copy link
Contributor

@Hipska can you rebase your branch on support/3.2 please ? So I can merge it.
(I just changed the target branch on the PR from develop to support/3.2)

@Molkobain Molkobain changed the title Add Brand logo and Model picture N°7644 - Add Brand logo and Model picture Jul 10, 2024
@Hipska Hipska requested a review from Molkobain July 10, 2024 08:26
@Molkobain Molkobain merged commit 7b91e9e into Combodo:support/3.2 Jul 10, 2024
@Molkobain
Copy link
Contributor

It's a merge! Thanks Thomas 🙌

@Hipska Hipska deleted the feature/brand_model_logo branch July 10, 2024 08:35
accognet pushed a commit that referenced this pull request Jul 11, 2024
@Hipska @accognet
N°7644 - Add Brand logo and Model picture (#646)
f9c9ccc 
* Add Brand logo and Model picture

* Add dictionary items for new attributes

* Add Model picture to summary card
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Molkobain Molkobain Molkobain approved these changes

Assignees

@Molkobain Molkobain

Labels
enhancement New feature or request
Projects
None yet
Milestone
3.2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@Hipska @Molkobain