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.

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

#9248 - Import/export an application context #9270

Merged
merged 9 commits into from
Jul 10, 2023

Conversation

dsuren1
Copy link
Contributor

@dsuren1 dsuren1 commented Jul 4, 2023

Description

This PR adds feature in context creator wizard to import/export the selected context.

The plugins are enabled by default under context-creator.

  • ContextImport
  • ContextExport

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Feature

Issue

What is the current behavior?

What is the new behavior?
User can export and import the context into context creator. Existing context will be replaced if any during import.
The name of the context is view is upon context import to avoid duplication

context_import_export

Breaking change

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • Yes, and I documented them in migration notes
  • No

Other useful information

@dsuren1 dsuren1 added this to the 2023.02.00 milestone Jul 4, 2023
@dsuren1 dsuren1 requested a review from offtherailz July 4, 2023 10:28
@dsuren1 dsuren1 self-assigned this Jul 4, 2023
@dsuren1 dsuren1 linked an issue Jul 4, 2023 that may be closed by this pull request
4 tasks
@dsuren1 dsuren1 enabled auto-merge (squash) July 4, 2023 12:01
Copy link
Member

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I tried to export a new context and it has a strange format. is there a reason for this?

    • It inclues a resource --> data tree containing the effective data to export
      • allTemplates should not be exported
      • pluginsConfig should not be exported too
        Please save only the content of resource --> data content in the root, consistent with the data saved on the server.
        Trying to import the context doesn't replace:
  • When importing a context on an existing one (navigating to the map and back before to import) the map is not updated
    screencast-drive.google.com-2023.07.05-16_09_44.webm

  • The import export buttons are available only on the first screen. I suggest to show them in all the windows (export is correct to be always available, import should be disabled on steps > 1. I suggest to disable it and show a tooltip saying "to import a context, go to step 1).

Other points:

  • the context name is not replaced. Anyway after disussion we agreed that not replacing it is the best solution

@dsuren1
Copy link
Contributor Author

dsuren1 commented Jul 6, 2023

@offtherailz
I have updated the PR with the rest of the comment

  • the context name (this is opinable anyway)

The reason for not replacing name is because, the name is unique, replacing it with imported value will cause a duplication which will force the user to modify it anyway. Importing on an existing context (saved context), the name is retained from the exisiting context so it works just like updating the context which will replace everything else except the name and id

@dsuren1 dsuren1 requested a review from offtherailz July 6, 2023 04:53
@offtherailz
Copy link
Member

@offtherailz I have updated the PR with the rest of the comment

  • the context name (this is opinable anyway)

The reason for not replacing name is because, the name is unique, replacing it with imported value will cause a duplication which will force the user to modify it anyway. Importing on an existing context (saved context), the name is retained from the exisiting context so it works just like updating the context which will replace everything else except the name and id

After discussion I updated the point you mention with the result of our discussion, avoiding to replace the name at all

Copy link
Member

@offtherailz offtherailz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • JSON still contains a "data" root. it have to be identical to the context JSON coming from the server. so something like { "windowTitle": "Save test", "mapConfig": {...}
  • As wel as I export and import map and windowTitle seems not to be imported correctly
screencast-github.com-2023.07.07-17_52_43.webm

Stopping here with the review. Please make sure that everything works fine before re-requesting the review.

@dsuren1 dsuren1 requested a review from offtherailz July 10, 2023 06:26
@dsuren1
Copy link
Contributor Author

dsuren1 commented Jul 10, 2023

@offtherailz
My bad, I stached last commit while cleanup and that's why I was wondering how these issues didn't happen to me while testing locally.

Ready for review now. Thanks!

@offtherailz
Copy link
Member

offtherailz commented Jul 10, 2023

I supposed it was something like this, It was too much strange 😄
Going to review 👍

@dsuren1 dsuren1 merged commit 2c86230 into geosolutions-it:master Jul 10, 2023
@dsuren1
Copy link
Contributor Author

dsuren1 commented Jul 17, 2023

@ElenaGallo
Kindly test this change in DEV. Thanks

@ElenaGallo
Copy link
Contributor

Hi @dsuren1,
shouldn't the context export be available only after entering both the name and the window title?
Now, starting from a new context, after entering the name it is possible to export it but when it is imported into a new context it is not recognized

import_export_context.mov

@dsuren1
Copy link
Contributor Author

dsuren1 commented Jul 17, 2023

@ElenaGallo We can but it behaves just like every other export tool making it consistent. ex: Dashboard where you can export a new dashboard without adding any widget. (Maybe we can enable it always too 🤔)

@offtherailz @tdipisa Kindly let me know your thoughts

@tdipisa
Copy link
Member

tdipisa commented Jul 17, 2023

@ElenaGallo @dsuren1

shouldn't the context export be available only after entering both the name and the window title?

Yes, as per my discussion with @dsuren1 today.

Now, starting from a new context, after entering the name it is possible to export it but when it is imported into a new context it is not recognized

we ca ignore this for the moment. The current behavior is in line with export of other resources. The export is taking the current state of a context and special cases, like the resource name check, seems already properly considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to import/export an application context
4 participants