Provide a report of the upgraded component when opening a script

[adecler]

NOTE: Depends on

BHoM/Versioning_Toolkit#159
BHoM/BHoM#1237
BHoM/BHoM_Engine#2495

Issues addressed by this PR

Closes #389

When a script is opened, if any component was upgraded through teh versioning toolkit, a dialog would pop-up with the details regarding the upgrades done to the script. Here's what it looks like:

Note that I've organised the data that way to facilitate the comparison of the old and new versions as it is sometimes tricky to spot what changed.

Test files

https://burohappold.sharepoint.com/:f:/r/sites/BHoM/02_Current/12_Scripts/02_Pull%20Request/BHoM/BHoM_UI/%23389-VersioningReport?csf=1&web=1&e=YMt3tB
The report above was generated using the Excel file from that folder. The hidden BHoM Sheet is a convenient way to test specific outdated items without the need to have a file containing that specific component. Best to test on other outdated scripts as well though.

[FraserGreenroyd]

I have tested this PR in conjunction with the PRs in the series across BHoM, Engine, Grasshopper, Excel, and Versioning (hopefully I didn't miss any).

I have performed the following tests:

• Run the Grasshopper test script provided by @adecler with successful results
• Run the Excel test script provided by @adecler with successful results
• Run 4 Grasshopper test scripts previously made by myself for versioning tests on other PRs with successful results

I do not have additional Excel test scripts, and have not attempted anything in the Dynamo UI.

On the whole, the functionality of the proposed work appears to be functioning nicely. I got similar pop up reports of my versioning changes to a script as described by @adecler in the top comment of this PR.

I have the following comments for discussion (I am happy for these to be put into new issues if they are outside the scope of this PR - if they are outside scope then I
am happy to do an approval on these PRs based on my tests):

• I was not able to copy the text on the report - notably where one versioning message was for a deleted component with no upgrade and it said "to raise an issue please go to blah" I found I could not highlight or copy the text of the blah which would prevent me (as a user) from copy/pasting the URL.
• The window does not appear to make it very clear to me that this is a BHoM upgrade only. It pops up and says the script is upgraded to version 4.2 and lists the components, but does not give an indication that, for example, only BHoM components/formulas have been affected. I think it would be beneficial to include the BHoM logo in the top-right (or other suitable location) of the report window, and use the BHoM favicon for the taskbar icon/top-left icon, along with an 'intro' line that makes it very clear that it's a BHoM window that is separate from the UI it's being used in.
• When versioning objects, are we able to give more information on what has changed? This may not be possible, but see screenshot below, it says it was upgraded from BH.oM.Environment.Elements.Space to a BH.oM.Environment.Elements.Space without saying what exactly is different when the object itself isn't (from memory it was a change of property names). As I say, not sure how much info you're getting from the versioning event (and how easy/quick it is to process) to display this, but capturing just in case it is possible.
• The Versioning Report does not close when Excel closes, but does when Rhino closes. I noticed that closing Rhino first closed the report window (expected behaviour in my opinion), but closing the parent Excel window did not close the report window, which stayed open in the background till it was manually closed. Potentially an edge-case user-issue given most people would probably close the report window first, but it's an inconsistent behaviour in terms of UX. Am happy if there is nothing to be done about this as I imagine Excel is playing a part and the effort to make it align is probably not worth the reward, but highlighting for reference anyway.

[adecler]

Thanks @FraserGreenroyd ,

• Copy text: Ha! the simple things are sometimes the more problematic. WinFrom labels do not support selection. TextBoxs do but they are not so great with the AutoSize. So you end up having to manually calculate the size of the box. This becomes problematic for those that have to wrap words on multiple lines. Funnily enough, dealing with web links is easier (at least assuming there is only one link). So I have added support for that.
• Making it clear this is just for the BHoM: Added icon and more specific text.
• Same description for old and new: YEs, that one is more tricky. Because a type is only described by its namespace + name. And I don't want to start adding things related to the input/outputs of a component inside the versioning engine to avoid having engine depending on UI. I could add some generic thing like (properties changed) if you want.
• Window not closing on Excel: That is weird! it closes fine for me. IT might be related to Excel sill not closing properly sometimes. Can you try a few more times to make sure. If it keeps happening, we should do a screen share.

[adecler]

@BHoMBot check core
@BHoMBot check copyright-compliance
@BHoMBot check versioning

[bhombot-ci]

@adecler to confirm, the following checks are now queued:

• core
• copyright-compliance
• versioning

There are 6 requests in the queue ahead of you.

[FraserGreenroyd]

Have re-reviewed following the changes and have the following comments:

• Copy text: Ha! the simple things are sometimes the more problematic. WinFrom labels do not support selection. TextBoxs do but they are not so great with the AutoSize. So you end up having to manually calculate the size of the box. This becomes problematic for those that have to wrap words on multiple lines. Funnily enough, dealing with web links is easier (at least assuming there is only one link). So I have added support for that.

This worked well and made it easy enough to do the crucial action where links are involved. I can imagine possible scenarios where multiple links are included, but I think we can include support for that at a later date, happy for that to be done as a small injection.

• Making it clear this is just for the BHoM: Added icon and more specific text.

Think this works well now with the logo and additional use of BHoM in the description.

• Same description for old and new: YEs, that one is more tricky. Because a type is only described by its namespace + name. And I don't want to start adding things related to the input/outputs of a component inside the versioning engine to avoid having engine depending on UI. I could add some generic thing like (properties changed) if you want.

I feel a generic message along the lines of the properties of this object have been updated. would be beneficial to help highlight what the versioning change is - but am happy for that to be a later injection to avoid too much work to this PR scope? Will let you judge that.

• Window not closing on Excel: That is weird! it closes fine for me. IT might be related to Excel sill not closing properly sometimes. Can you try a few more times to make sure. If it keeps happening, we should do a screen share.

As I tried again I got this. Long time no see... not sure if it's linked to this PR, or Excel generally as it has been a very long time since I used BHoM in Excel - I am also happy for this to not hold up this PR as this may be machine specific, or out of scope of what this PR is introducing. The report itself renders fine, and I'm sure the odds of someone closing Excel before closing the versioning report are slim (though admittedly not zero).

If you're happy @adecler I'm happy to approve all the PRs in this series with an aim to merge in and get some alpha testing done in the next few weeks - I'll let you decide regarding the message for objects with properties as to whether you want to add to this PR or raise an issue for later. Same with multiple links. If you opt to go for merge now, if you want to handle any outstanding check requests (I won't do it now in case you decide to push commits, just to save the computing power 😄 ) and I can approve/merge in the morning (my time). Alternatively if you opt to push some updates I'll run another round of tests and then approve/merge as appropriate 😄

[adecler]

Thanks @FraserGreenroyd ,

I have added the generic message on objects that have updated properties. I have also handled the issue of Excel crashing when closing by managing the thread explicitly when a workbook is closed. It looks like Excel was a bit more picky than Grasshopper on that regard.

[adecler]

@BHoMBot check required

[bhombot-ci]

@adecler to confirm, the following checks are now queued:

• code-compliance
• documentation-compliance
• project-compliance
• core
• null-handling
• serialisation
• installer
• versioning

[bhombot-ci]

@adecler just to let you know, I have provided a check-versioning result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @adecler on BHoM

[bhombot-ci]

FAO: @FraserGreenroyd
@adecler is seeking dispensation on this Pull Request to skip a required check. Please can you provide authorisation for the check to be skipped, or provide assistance as appropriate.

The check they wish to have dispensation on is project-compliance.

If you are providing dispensation on this occasion, please reply with:

@BHoMBot this is a CI/CD instruction. I am authorising dispensation to be granted on check ref. 2572415398

[adecler]

@FraserGreenroyd , it looks like the bot is confused between Versioning engine and the Version keyword in dll references.

[adecler]

@BHoMBot check copyright-compliance
@BHoMBot check core
@BHoMBot check versioning
@BHoMBot check installer

[bhombot-ci]

@adecler to confirm, the following checks are now queued:

• copyright-compliance
• versioning
• installer

[adecler]

@BHoMBot check copyright-compliance
@BHoMBot check core
@BHoMBot check versioning
@BHoMBot check installer

[bhombot-ci]

@adecler to confirm, the following checks are now queued:

• copyright-compliance
• core
• versioning
• installer

[bhombot-ci]

@adecler just to let you know, I have provided a check-versioning result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @adecler on BHoM_Engine

[bhombot-ci]

@adecler just to let you know, I have provided a check-versioning result to this Pull Request as it was detected to be linked to other Pull Requests in a series. The comment which triggered this check came from @adecler on BHoM

[FraserGreenroyd]

Thanks @FraserGreenroyd ,

I have added the generic message on objects that have updated properties. I have also handled the issue of Excel crashing when closing by managing the thread explicitly when a workbook is closed. It looks like Excel was a bit more picky than Grasshopper on that regard.

Have seen the generic message and am happy with it.

Excel also closed nicely instead of having a hissy-fit 😄

I have followed the same test I outlined in my first review and am happy with the outcome, and cannot find any more things to improve at this stage (I reserve the right to find things next week 😝 ). As such, I'm approving this PR and all PRs in the series.

I note that we're not waiting for many more reviews based on the reviewer list across the PR series, so I'll merge these in now so that we get an alpha tomorrow with this included for others to take a look at via the alpha artefacts.

@adecler I'll leave to you to post some comms about this addition on Teams/Yammer/Slack as appropriate if you wish. We should probably look to do a review session on this Engine PR as well shortly before we get too close to beta.

[FraserGreenroyd]

See this comment