search

netcreateorg / netcreate-itest

Developing the 2.0 version of NetCreate
https://github.com/netcreateorg/netcreate-2018
Other
0 stars 0 forks source link

Edge delete button doesn’t immediately appear #125

Closed jdanish closed 5 months ago

jdanish commented 5 months ago

To reproduce:

create an edge but don’t set the Save Edit edge No delete Reload screen and go edit edge The delete button is now there

jdanish commented 5 months ago

Upon additional editing it appears the issue is that edges created by import are not being flagged as saved, so if you edit, then save, then delete appears.

benloh commented 5 months ago

The issue here is that the "Delete" button is only shown if the edge has been saved (and bumps the revision number to 0). This is needed so that a newly created edge does NOT have show a delete button.

The problem is that importing data does not update the revision value, so the system treats the edge as if it's a newly created unsaved edge. The solution is probably to save any imported edge that does not define a revision number to be revision 0.

benloh commented 5 months ago
benloh commented 5 months ago

@jdanish I think this raises a general question of how we want to handle the revision field. It's a little odd because it's more of a "system" field that is mostly not visible (though you can see view it under Provenance). There is currently a bit of a disconnect with how we're handling the field: the problem basically comes down to whether you want revision to be tracked during exports and imports.

Currently when you export data, the revision field is NOT being included in the export. This means that when you import data, the revision is not being imported either (unless you manually add that field to the import csv table). The result is that the imported data record keeps the default revision of -1 aka "unsaved". (This is why the "Delete" button is hidden -- the system thinks this is a newly-created unsaved edge, so it doesn't show the "Delete" button).

Some questions:

  1. Would you want to export the revision field when you export nodes or edges? Would this be confusing to have that field included there when it's kind of a "system" field?
  2. When you import new nodes/edges, and the revision field is not defined, should we default the revision to '0' (e.g. saved one time)?
  3. If we DON'T export the revision field, and you export, then reimport data, any nodes/edges that were exported and re-imported would revert to a revision of 0.

If you don't care about revision, the simple fix is to set revision to 0 on import. If you want to track revision across imports and exports, then we need to make sure we export and import it.

jdanish commented 5 months ago

I believe this is mostly just for "while editing" and anything that is imported could safely be treated as saved and thus deletable. So we wouldn't need to import / export revision. I certainly can't think of a case where I'd want to import a field but have it not really delete and behave oddly?

The other thought I have is that this should likely synergize with the long-term thinking on provenance. Do we want to require provenance be imported / exported? In which case revisions are part of that?

@kalanicraig any thoughts?

kalanicraig commented 5 months ago

I do like the idea of revisions being part of the export, for analysis purposes. Would we be incorporating info from logs to do that? If so, would that mean we wouldn’t need to have provenance be part of the import? Or are we thinking about provenance as an importable field as a sort of frankenstein’s-monster field compiled from both automatic revision tracking and manual provenance notes?

On Mon, Jan 22, 2024 at 7:32 PM Joshua Danish @.***> wrote:

I believe this is mostly just for "while editing" and anything that is imported could safely be treated as saved and thus deletable. So we wouldn't need to import / export revision. I certainly can't think of a case where I'd want to import a field but have it not really delete and behave oddly?

The other thought I have is that this should likely synergize with the long-term thinking on provenance. Do we want to require provenance be imported / exported? In which case revisions are part of that?

@kalanicraig https://github.com/kalanicraig any thoughts?

— Reply to this email directly, view it on GitHub https://github.com/netcreateorg/netcreate-itest/issues/125#issuecomment-1905078319, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACKL4NEHZ7NUWQ77V7DDOILYP4AJHAVCNFSM6AAAAABCEACY6OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMBVGA3TQMZRHE . You are receiving this because you were mentioned.Message ID: @.***>

benloh commented 5 months ago

The simplest thing might be to:

So if you never export provenance, it won't replace your data. But if it's present, we replace it. The other approach might be to add a Use provenance checkbox so that you can turn it on and off for both import and export?

benloh commented 5 months ago

See #127 for the fix.