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

Import feature: show the differences of an element when it is updated by an import (and refactor) #810

Merged
merged 205 commits into from
Jul 12, 2024
Merged

Import feature: show the differences of an element when it is updated by an import (and refactor) #810

merged 205 commits into from
Jul 12, 2024

Conversation

MyPyDavid
Copy link
Member

@MyPyDavid MyPyDavid commented Nov 1, 2023 •
edited
Loading

Description

This PR implements a text difference viewer for the import of RDMO content elements (catalogs, optionsets, etc..).
It uses the react-diff-viewer-continued to display the changes during an import. In the back-end a sort of "track changes" function tracks the current and new state of each element which is returned in the json response.
The code architecture of the import is refactored into one central import function under management with helpers for each type of element.

Related issues: #468, #469

Includes bug fix for: #998

Motivation and Context

How has this been tested?

Screenshots (if appropriate)

Currently:

management-import-optionsets-1-changes

previous version ![management-import-optionsets-1-changes](https://github.com/rdmorganiser/rdmo/assets/13996213/e5fcb01c-bc8e-4c86-9b89-4318fd015c47)
older previous version ![management-import-optionsets-1-changes](https://github.com/rdmorganiser/rdmo/assets/13996213/afec9fae-839b-496f-a817-5329f4167a45)

Types of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Code style update (formatting, renaming)
    • some type hints have been added to the new code
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Checklist

  • I have read the contributor guide.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
    • also added more front end e2e tests
  • All new and existing tests passed.

Still todo

Front-end

  • add filter on top of import page
  • add info messages summary on top of import page
  • button to show only the changed elements

@jochenklar jochenklar added this to the 2.2.0 milestone Nov 24, 2023
@jochenklar jochenklar changed the base branch from dev-2.1.0 to dev-2.2.0 December 12, 2023 13:46
@MyPyDavid MyPyDavid changed the title Show the differences of an element when it is updated by an import Show the differences of an element when it is updated by an import (and refactor import) Apr 4, 2024
@MyPyDavid MyPyDavid marked this pull request as ready for review April 11, 2024 09:52
@MyPyDavid
Copy link
Member Author

MyPyDavid commented Apr 26, 2024 •
edited
Loading

  • fix the actions in the sidebar

@MyPyDavid MyPyDavid changed the title Show the differences of an element when it is updated by an import (and refactor import) Import feature: show the differences of an element when it is updated by an import (and refactor) May 15, 2024
jochenklar
jochenklar requested changes May 16, 2024
View reviewed changes
package-lock.json Outdated Show resolved Hide resolved
rdmo/conditions/imports.py Outdated Show resolved Hide resolved
rdmo/core/constants.py Show resolved Hide resolved
rdmo/core/constants.py Show resolved Hide resolved
rdmo/core/import_helpers.py Outdated Show resolved Hide resolved
rdmo/core/xml.py Outdated Show resolved Hide resolved
rdmo/management/imports.py Show resolved Hide resolved
rdmo/management/imports.py Outdated Show resolved Hide resolved
rdmo/management/imports.py Outdated Show resolved Hide resolved
rdmo/management/imports.py Show resolved Hide resolved
@jochenklar
Copy link
Member

I am sorry, there is more work to do on the css. Paddings and margins are off all over the place.

@jochenklar
Copy link
Member

jochenklar commented May 16, 2024 •
edited
Loading

Screenshot_2024-05-16_15-08-24

The padding needs to align with the rest of the panel.

I think we should only use the <code> for URIs or Element/Model names. Just Normal font should suffice. Also the "Changed:" label in the front is redundant.

jochenklar and others added 10 commits July 3, 2024 11:34
@MyPyDavid
Copy link
Member Author

rebased to dev-2.2.0 and force pushed

@jochenklar
Copy link
Member

I think new is great since we use new/neu also in the interface.

@MyPyDavid
Copy link
Member Author

ok, changed the text to "new"
and added (new from dev-220) the short_title field to the import helpers

@jochenklar
Copy link
Member

Sorry I found more bugs:

  • if you upload and import a faulty file the message is missleading:

image

with this xml

<?xml version="1.0" encoding="UTF-8"?>
<rdmo xmlns:dc="http://purl.org/dc/elements/1.1/" created="2024-07-01T10:53:07.185903+02:00" version="2.2.0.dev1">
	<question dc:uri="http://example.com/terms/questions/catalog/blocks/set/block/b">
		<uri_prefix>http://example.com/terms</uri_prefix>
	</question>
</rdmo>

(GitHub does not allow XML uploads)

  • For the same file: I got this

image

after I click Question or click show. I think this is also because of the broken xml file, so it is not super important.

  • I would remove the onClick on the created/updated badge or add cursor: pointer; using css, upt to you.

@MyPyDavid
Copy link
Member Author

MyPyDavid commented Jul 8, 2024 •
edited
Loading

thanks for noticing and the xml.

* if you upload and import a faulty file the message is missleading:

This came from the back-end. I've changed it now so that "new" and "create" will not show at import when there are errors (and the db instance won't be saved anyways).

* I would remove the onClick on the created/updated badge or add cursor: pointer; using css, upt to you.

I've removed the onClick from the badge.

One other issue with this faulty xml was that the buildUri doesn't function properly anymore (it shows ../undefined in the URI).
I've changed it now so that the front-end that it will keep the element.uri when buildUri can not build the URI and returns null.

Another mini-feature addition I thought about was, to make these URIs at import also clickable (so that they open the edit page of each element, like in management). I would just need to the element.id from the backend and build the link. Do you think I should add that here?

@MyPyDavid MyPyDavid requested a review from jochenklar July 10, 2024 08:30
@jochenklar
Copy link
Member

No I would not add that. It could lead to problems if the users then go back in the browser. They should just click on "Back".

@MyPyDavid
Copy link
Member Author

MyPyDavid commented Jul 11, 2024 •
edited
Loading

  • update the dots in error messages

@MyPyDavid MyPyDavid linked an issue Jul 12, 2024 that may be closed by this pull request
colorize the error/warning messages during import #469
Closed
@MyPyDavid MyPyDavid merged commit d62ec2b into dev-2.2.0 Jul 12, 2024
17 checks passed
MyPyDavid added a commit that referenced this pull request Jul 12, 2024
@MyPyDavid
fix: #810 fix xml import of additional_input
a77fc23 
Signed-off-by: David Wallace <[email protected]>
@MyPyDavid MyPyDavid deleted the feat-import-diff branch August 22, 2024 07:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CalamityC CalamityC CalamityC approved these changes

@jochenklar jochenklar jochenklar approved these changes

Assignees

@MyPyDavid MyPyDavid

Labels
status:acceptance type:feature-request
Projects
None yet
Milestone
RDMO 2.2.0
3 participants
@MyPyDavid @jochenklar @CalamityC