archimatetool / archi-portico-plugin

Archi plugin to import/merge a reference model into another model
6 stars 2 forks source link

Merge Views #2

Closed Phillipus closed 4 years ago

Phillipus commented 4 years ago

Merge Views.

Phillipus commented 4 years ago

"source model" = model that is imported and merged "target model" = model that receives the imported model

I've implemented the first part of this - if a View in the source model does not exist in the target model it, and its contents are created.

To do - updating an existing View.

How should we do this? Would it be easier to simply replace the target View with the source View and do integrity checks afterwards?

jbsarrodie commented 4 years ago

Testing...

I've implemented the first part of this - if a View in the source model does not exist in the target model it, and its contents are created.

I've tested successfully using my small A/B models and with "update" mode disabled. But I've also tested with two (big and confidential) models: at first it seems to be good, but when trying to save the model, it fails with this error message: image

Investigating ATM

To do - updating an existing View. How should we do this?

The target behavior is the following:

  1. If update mode is off:
    1. if a view is being imported and doesn't exist in target, then create the view
    2. if a view is being imported and exists in target, don't import it
  2. If update mode is on:
    1. if a view is being imported and doesn't exist in target, then create the view (same as "1.i")
    2. if a view is being imported and exists in target, the one from source should replace the one from target.

A simple way to do "2.ii" is to remove the view in target and then import it like in "2.i", but this would lead to this view being removed from other views that would reference it. So a best way is to keep the view, restore its name, properties and documentation, and remove its content and import it from source.

Would it be easier to simply replace the target View with the source View and do integrity checks afterwards?

Not sure we can do it afterward (or even not sure there is something to check afterward): When doing "1.i", we can run in one edge case: importing a view which references a relationship whose ends have changed since a previous import. Because "update mode" is off, we are trying to create a connection related to a relationship which doesn't match the source and target nodes. This should be traced and summarized in a warning message at the end. (in #1, this is the first edge case mentioned)

Re #1, the second edge case is not related to view import, but is a bit more difficult to catch because it involves checking if a relationship being updated had its ends changed after a first import done in the past, and check if this relationship is used in some views: these views will obviously be impacted by the relationship being "restored" (ie. the relationship will be removed from views). This should be traced and summarized in a warning message at the end.

jbsarrodie commented 4 years ago

But I've also tested with two (big and confidential) models: at first it seems to be good, but when trying to save the model, it fails with this error message:

This happens when the relationship involves another relationship. Maybe you first import elements and then only relationship, assuming that then relationships can be imported without issues, but two passes (or some other trick) should be used.

jbsarrodie commented 4 years ago

This happens when the relationship involves another relationship.

Quick workaround: do import twice ;-)

BTW, I've noticed a bug that I'll trace in #1

Phillipus commented 4 years ago

A simple way to do "2.ii" is to remove the view in target and then import it like in "2.i", but this would lead to this view being removed from other views that would reference it. So a best way is to keep the view, restore its name, properties and documentation, and remove its content and import it from source.

The latest commit does exactly this. Another reason to keep the View as you suggested is if it is open in the UI - the opened View is not closed and updated so we end up with two. Anyway, your idea works.

Now we need to test for those edge cases and also check whether there are any issues with view references in other views...

jbsarrodie commented 4 years ago

Great. I'll test asap and let you know. Thank you!

Phillipus commented 4 years ago

restore its name, properties and documentation

Just to check - you mean replace the target name, properties and documentation with the imported ones, right?

jbsarrodie commented 4 years ago

Just to check - you mean replace the target name, properties and documentation with the imported ones, right?

Yes, and features too

Phillipus commented 4 years ago

OK, that's done then,

Phillipus commented 4 years ago

~Note - images from Canvas Views need to be imported.~

Done.

jbsarrodie commented 4 years ago

FWI, I did import a model inside another ('update' mode off) and all concepts and views (including canvas with images) were successfully imported.

I sill have to test edge cases involving updates...

Phillipus commented 4 years ago

I think the two edge cases are taken care of in ModelImporter#SetArchimateReconnectionCommand which will try to reconnect any changed connection ends in Views or delete connections no longer valid. So it's now a case of how we want to inform the user that this occurred.

Phillipus commented 4 years ago

So it's now a case of how we want to inform the user that this occurred.

But perhaps what might be useful after the import is a "WTF just happened?" status report. So we could throw up a dialog something like:

Import successful
No changes
Import successful
12 Elements added, 5 updated
1 Folder updated
2 Views added, 1 updated
2 Connections deleted, 1 re-connected

And maybe go into more detail with names of concepts, folders, views.

I don't know if one would want more detail than that?

jbsarrodie commented 4 years ago

Closing this one as import of views is ok and other topics have dedicated issues.