archimatetool / archi

Archi: ArchiMate Modelling Tool
https://www.archimatetool.com
MIT License
914 stars 267 forks source link

Show used-in-views and model-relations in html-reports #835 #984

Closed janesser closed 8 months ago

janesser commented 8 months ago

wip = work in progress

835

Phillipus commented 8 months ago

Hi, thanks for the PR. However, before submitting a PR could you open an issue in the issue tracker with a summary of what the change is for. This means we can discuss whether it is a desired feature and how it fits in with the roadmap. Thanks!

Edit: OK I see a reference to #835

janesser commented 8 months ago

@Phillipus i feel like it would worth a first look. let me know what you think.

Phillipus commented 8 months ago

Thanks for the changes. I think the best thing to do now is if you could thoroughly test it. :-) Test elements that aren't in views, elements nested in other objects, etc...

jbsarrodie commented 8 months ago

Hi guys,

@janesser I'll look at all this later today, but why did you had to change java code? Everything should be doable from the template itself by using ArchiMateConcept's API (ie IArchimateConcept.getSourceRelationships(), IArchimateConcept.getTargetRelationships() and IArchimateConcept.getReferencingDiagramComponents()) and iterating on returned list.

Phillipus commented 8 months ago

IArchimateConcept.getReferencingDiagramComponents() shouldn't be used here. DiagramModelUtils.findReferencedDiagramsForArchimateConcept(element) should be used. I don't know if that can be accessed from the template.

janesser commented 8 months ago

Thanks for the changes. I think the best thing to do now is if you could thoroughly test it. :-) Test elements that aren't in views, elements nested in other objects, etc...

see attached the cases i tested throughout the process. Additionally tried several variations like relations with and without name, and empty lists in the affected spots.

Let me know if there are blindspots left.

test.archimate.zip

jbsarrodie commented 8 months ago

IArchimateConcept.getReferencingDiagramComponents() shouldn't be used here.

Well, it can be used without any issue. I've just created a POC code for that:

In original frame.stg, edit around line 233:

    ^endif^
    <div class="convertlinks documentation" style="white-space:pre-wrap">^element.Documentation;format="xml-encode"^^element.Purpose;format="xml-encode"^^element.Content;format="xml-encode"^</div>

    <!-- JB's Hack Starts Here -->
    <br><b>Used in views:</b><br>
    <ul id="usedinviews">
    ^if(element.Documentation)^
        ^element.ReferencingDiagramComponents:{dmc | <li>^dmc.DiagramModel.Name^</li>}^
    ^endif^
    </ul>
    <!-- JB's Hack Ends Here -->
</div>
<div role="tabpanel" class="tab-pane" id="properties">

This appends the list of views to the documentation. OF course, if a concept is shown more than once in a view, this view will appear several times in the list. But, as we have to use javascript to sort the list anyway, then adding this to frame.jsaround line 163 does the trick:

    var tabPropertiesRows = tabProperties.children('tr');
    tabPropertiesRows.sort(strcmp).appendTo(tabProperties);

    // JB's Hack Starts Here
    var usedInViews = $('#usedinviews');
    uniqueLi = {};
    usedInViews.children('li').each(function () {
      var thisVal = $(this).text();

      if ( !(thisVal in uniqueLi) ) {
        uniqueLi[thisVal] = "";
      } else {
        $(this).remove();
      }
    })
    var usedInViewsList = usedInViews.children('li');
    usedInViewsList.sort(strcmp).appendTo(usedInViews);
    // JB's Hack Ends Here

    const type = document.location.href.split('/').slice(-2, -1).pop();
    if (type == "views") {

So no reason to change a single line of java code IMHO.

BTW, one of my design principle when working on the HTML report was to avoid having to add new logic into the java code to support a new feature in the report itself. That's one of the biggest issue with the way JasperReport lib works: the reports fully delegates the logic to the java code, which makes it very hard to extend. While with the HTML report, StringTemplates allows us to navigate the model, and the javascript code makes it possible to sort, filter...

Phillipus commented 8 months ago

@jbsarrodie Thanks for the guidance. Because I didn't work on the original code for this report I was unaware that you could do this!

jbsarrodie commented 8 months ago

I was unaware that you could do this!

StringTemplate is really a nice library on this aspect. Quite powerful.

@janesser Could you please do a test in which you use the original java code and only extend the frame.stg by iterating on element.SourceRelationships, element.TargetRelationships and element.ReferencingDiagramComponents ?

janesser commented 8 months ago

@jbsarrodie All proposals adopted and tested.

StringTemplate would not let me use any kind of comment like ^! or <! not sure why. Anyways would have been the first to comment in frame.stg.

Are we good to merge ? EDIT: little fix was left

Phillipus commented 8 months ago

Hi, I left a comment at one line of the code. Also, the "Used in Views" section is no longer working. Did you test?

Phillipus commented 8 months ago

Hi, I'm still not seeing views in "Used in Views". I only see a link to the selected element.

Phillipus commented 8 months ago

element.ReferencingDiagramComponents returns the diagram objects but not the view that they are in. You then need to use v.DiagramModel. Look at JB's POC code above.

janesser commented 8 months ago

i need summarize, which way led here, and what are possible continuations:

thinking forward, i can

@jbsarrodie @Phillipus it's on you.

Phillipus commented 8 months ago

This seems to work:

<td><a href="../views/^v.DiagramModel.Id^.html" target="view" title="^v.DiagramModel.Name;format="xml-encode"^"> ^v.DiagramModel.Name;format="xml-encode"^ </a></td>

Phillipus commented 8 months ago

That works now.

But now:

This appends the list of views to the documentation. OF course, if a concept is shown more than once in a view, this view will appear several times in the list. But, as we have to use javascript to sort the list anyway, then adding this to frame.jsaround line 163 does the trick:

To test this, create a new View. Add a concept to it twice so there are two instances.

janesser commented 8 months ago

I added jquery deduplication - no sorting atm. i guess there are off-the-shelf js-frameworks for data-tables.

p.s.: actually jquery is still fun.

jbsarrodie commented 8 months ago

i need summarize, which way led here, and what are possible continuations:

  • the java-code was fully functional and thoroughly tested
  • redoing the thing in a pure stringtemplate ships a few disadvantages - like loss of type-control - navigating object-structures without guards - continue the list as you like. the advantage IMHO is to have it all in one place.

Thank you very much for the (positive) test of the "StringTemplate only" solution.

@jbsarrodie @Phillipus it's on you.

My personal preference is for the "StringTemplate only" solution.

I guess there are off-the-shelf js-frameworks for data-tables.

You can do a basic sort with 3 lines of code, thanks to jQuery. You have the example just before the code you added:

var tabProperties = $('#properties > table > tbody');
var tabPropertiesRows = tabProperties.children('tr');
tabPropertiesRows.sort(strcmp).appendTo(tabProperties);

p.s.: actually jquery is still fun.

I do agree ;-)

janesser commented 8 months ago

You can do a basic sort with 3 lines of code, thanks to jQuery. You have the example just before the code you added:

var tabProperties = $('#properties > table > tbody');
var tabPropertiesRows = tabProperties.children('tr');
tabPropertiesRows.sort(strcmp).appendTo(tabProperties);

i am afraid i would compare to html-strings here, where it would get sorted by their uuid eventually.

getting a real sort-by-name, and maybe even a sort-by-perspective sounds interesting to me. In fact, having one uniform+generic way of dealing with tables in the html-report sounds like dedicated mission to me. this is supported by the crosscheck of other tables already contained in the html report.

tomorrow ill have a look to polish up this one, which should not be the last PR on mighty archi.

janesser commented 8 months ago

polishing adjustments:

@jbsarrodie @Phillipus What's left ?

Phillipus commented 8 months ago

@jbsarrodie @Phillipus What's left ?

I guess some thorough testing?

Thanks for the work you have done on this!

Phillipus commented 8 months ago

Hi, the Viewpoint name is not working, I see "No Viewpoint" in all cases. I think line 273 should be:

<td><span class="i18n-viewpoint-^v.DiagramModel.Viewpoint^" /></td>
Phillipus commented 8 months ago

Thanks for the fix. I've done some testing on this now, @janesser if you could confirm all the testing now we could merge this.

janesser commented 8 months ago

@Phillipus thanks for the second pair of eyes. We are good to merge from my perspective.

Phillipus commented 8 months ago

I've merged into master branch now. If we discover any further issues we can fix later.

@janesser Thank-you for your work on this, I know it hasn't been easy. ;-)

In case you do any further PRs I suggest - (1) open an issue for discussion first before submitting a PR (in this case the issue was already open, I simply missed it) and (2) Better to create a dedicated branch in your fork rather than master as it's easier to work with (and force push to).

👍

janesser commented 8 months ago

I've merged into master branch now. If we discover any further issues we can fix later.

@janesser Thank-you for your work on this, I know it hasn't been easy. ;-)

In case you do any further PRs I suggest - (1) open an issue for discussion first before submitting a PR (in this case the issue was already open, I simply missed it) and (2) Better to create a dedicated branch in your fork rather than master as it's easier to work with (and force push to).

👍

Pleasure is all mine ;-)

Really enjoyed delving back in software craftmanship, and i have been in excellent company of you guys @Phillipus and @jbsarrodie .

Thanks for the suggestions. No promises, but that is probably not my last one.

Now it's up to deploying the use of that analysis-tab in some disciplines' routines, in my probably-not-so-specific case pivoting across project-portfolio.