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

GNIP-84: Upload Page Enhancements #7154

Closed
3 of 5 tasks
afabiani opened this issue Mar 24, 2021 · 4 comments
Closed
3 of 5 tasks

GNIP-84: Upload Page Enhancements #7154

afabiani opened this issue Mar 24, 2021 · 4 comments
Assignees
Labels
gnip A GeoNodeImprovementProcess Issue
Milestone

Comments

@afabiani
Copy link
Member

afabiani commented Mar 24, 2021

GNIP-84: Upload Page Enhancements

Overview

The way uploads are currently managed by GeoNode through the “Upload Layers” form is weak and inconsistent most of the time. There are plenty of issues mostly due to a wrong architectural choice, not allowing to correctly manage the “Upload Sessions”.

The proposal is to improve a bit this mechanism in order to make this more robust.

Proposed By

@afabiani [email protected]
@giohappy [email protected]

Assigned to Release

This proposal is for GeoNode 3.2.0.

State

  • Under Discussion
  • In Progress
  • Completed
  • Rejected
  • Deferred

Motivation

The way uploads are currently managed by GeoNode through the “Upload Layers” form is weak and inconsistent most of the time. There are plenty of issues mostly due to a wrong architectural choice, not allowing to correctly manage the “Upload Sessions”.

Proposal

Upload Sessions are not treated as stateful

While uploading a dataset by using the Upload form, an object will be created into the DB in order to keep the state of the session, along with the uploaded files and user choices, among the upload workflow.

Nevertheless, the UploadSession is not treated as stateful by the Upload form. This causes a set of severe side effects:

  1. Whenever something happens during the upload, there won’t be any way to recover the Upload Session or clean up the inconsistent states and partially uploaded files.

  2. It won’t be possible to finalize the upload of more than one dataset requiring additional configuration steps, e.g.:

    image
    In this example here, it won’t be possible to finalize the upload of all temporal layers, but only one.

    All the files that have been previously uploaded will be lost.

    If the page expires or gets refreshed for some reason, there won’t be any way to recover the uploads.

  3. Partially configured datasets appear among the layers’ list, giving the impression to the user that something went wrong.

Technical Proposal

The proposal here would be to make the Upload page stateful and aware of the ongoing Upload sessions, similar to what we already do with the Remote Services.

When harvesting a Remote layer, we take benefits from the asynchronous tasks by following their progress and having the possibility of issue again a failed harvesting job (see the figure below)
image

The idea here would be to improve the Upload Form in order to be able to read the list of the currently active Upload Session, follow their progress and allow the user to either finalize the workflow or cancel it and consequently make GeoNode able to clean up the pending, unfinished resources.

Moreover the proposal would also like to avoid publishing resources before the whole upload process has finished and notify the user about some pending upload in progress.

By making the Upload form stateful, it would be possible also to allow a superuser to control the pending uploads from other users and take action before the upload actually finishes or recover from broken ones.

Mockups of the Upload Form Improvements

Notify the user about pending uploads

image
An immediate visual marker will make the user aware of still ongoing uploads.

Stateful Upload Sessions

image

Notes: We must define a criteria to determine which completed upload should be visible in the list. They cannot disappear immediately, otherwise the user won't understand, but we cannot keep them forever of course. Using a “notification” approach (keeping track of dismiss requests from the user, or the number of times the notification has been shown) would require more work which isn’t worth it now; 1h from upload completion time could be a good good compromise. However if the user uploads something big, goes away and get back to that page after more then 1 hour, he won’t see the completed layer notification...

Backwards Compatibility

Compatible with 3.2.0 and 3.1.2.

Future evolution

Complete refactor of the GeoNode Upload Workflow.

Feedback

Update this section with relevant feedbacks, if any.

Voting

Project Steering Committee:

  • Alessio Fabiani: 👍
  • Francesco Bartoli: 👍
  • Giovanni Allegri: 👍
  • Simone Dalmasso:
  • Toni Schoenbuchner: 👍
  • Florian Hoedt: 👍

Links

Remove unused links below.

@afabiani afabiani self-assigned this Mar 24, 2021
@afabiani afabiani added the gnip A GeoNodeImprovementProcess Issue label Mar 24, 2021
@afabiani afabiani added this to the 3.2 milestone Mar 24, 2021
@t-book
Copy link
Contributor

t-book commented Mar 24, 2021

Thanks Alessio. my +1

@gannebamm
Copy link
Contributor

+1 with a minor question:

Using a “notification” approach ([..]) would require more work which isn’t worth it now [..]

The current system will notify other group members that a new dataset was uploaded via eg email and will list it in the social-activities stream. The new system will not use those?

@francbartoli
Copy link
Member

+1 @afabiani.

Just want to check if you have a timeline for this and the overall 3.2. That would help me with a project.

Thanks!

@afabiani
Copy link
Member Author

@francbartoli as soon as this GNIP is finished, we would like to finalize the release. Hopefully we could be able to finalize this one this week.

afabiani pushed a commit that referenced this issue Apr 8, 2021
* - Fix the "Upload" models fields and correctly link it to the "GeoNode Layer"

* - Adding REST Interfaces for the "Upload" models

* - [WIP] Upload api test cases

* - [WIP] Hide non processed resources

* - [WIP] Management of the Importer Session during Resume

* - [WIP] Hide dirty resources to everyone / Make sure Upload delete cascade is invoked everytime

* - [WIP] Update the UploaderSession state everytime we reload the importer session

* - [WIP] Set Uploader Session state to INVALID if any error occurs

* - [WIP] Get rid of incomplete uploads on templates that are not related to layer uploads

* - [WIP] Exposing Upload resume, delete and import URLs via REST APIs

* update workflow and ui of incomplete upload list

* replace import_id with id for the request to the api

* improve link button behaviour

* - Adapt failing test cases to the new "dirty_state" filtering condition

* - [WIP] Do not throw an exception when creating the "resume_url"

* - [WIP] Move the "Layer Info" button to the upload list

* add pagination and remove modal on list of incomplete upload

* Revert " - Adapt failing test cases to the new "dirty_state" filtering condition"

This reverts commit bee78bd.

* - [WIP] Make the Upload return URLs "state" aware

* - [WIP] Handle upload POST via REST interface

* - [WIP] Make "resourcebase_api" use "get_visible_resources" more efficiently

 - General code optimization and test case fixing

* - [WIP] Do not delete the Upload associated Layer

* - [WIP] Adding Upload Test Cases

* - Test fixes

* - Test fixes

* - Adding Upload Selenium Live Tests

* add processed item to the upload table

* - [WIP] Adding "create_date" to the Upload session

* sort the table by created date

* - Test fixes

* - Updating translations

* - Test fixes

* - Test fixes

Co-authored-by: allyoucanmap <[email protected]>
@afabiani afabiani closed this as completed Apr 8, 2021
afabiani pushed a commit that referenced this issue Apr 8, 2021
* - Fix the "Upload" models fields and correctly link it to the "GeoNode Layer"

* - Adding REST Interfaces for the "Upload" models

* - [WIP] Upload api test cases

* - [WIP] Hide non processed resources

* - [WIP] Management of the Importer Session during Resume

* - [WIP] Hide dirty resources to everyone / Make sure Upload delete cascade is invoked everytime

* - [WIP] Update the UploaderSession state everytime we reload the importer session

* - [WIP] Set Uploader Session state to INVALID if any error occurs

* - [WIP] Get rid of incomplete uploads on templates that are not related to layer uploads

* - [WIP] Exposing Upload resume, delete and import URLs via REST APIs

* update workflow and ui of incomplete upload list

* replace import_id with id for the request to the api

* improve link button behaviour

* - Adapt failing test cases to the new "dirty_state" filtering condition

* - [WIP] Do not throw an exception when creating the "resume_url"

* - [WIP] Move the "Layer Info" button to the upload list

* add pagination and remove modal on list of incomplete upload

* Revert " - Adapt failing test cases to the new "dirty_state" filtering condition"

This reverts commit bee78bd.

* - [WIP] Make the Upload return URLs "state" aware

* - [WIP] Handle upload POST via REST interface

* - [WIP] Make "resourcebase_api" use "get_visible_resources" more efficiently

 - General code optimization and test case fixing

* - [WIP] Do not delete the Upload associated Layer

* - [WIP] Adding Upload Test Cases

* - Test fixes

* - Test fixes

* - Adding Upload Selenium Live Tests

* add processed item to the upload table

* - [WIP] Adding "create_date" to the Upload session

* sort the table by created date

* - Test fixes

* - Updating translations

* - Test fixes

* - Test fixes

Co-authored-by: allyoucanmap <[email protected]>
(cherry picked from commit 14070b2)

# Conflicts:
#	geonode/api/tests.py
#	geonode/locale/af/LC_MESSAGES/django.po
#	geonode/locale/af/LC_MESSAGES/djangojs.po
#	geonode/locale/al/LC_MESSAGES/django.po
#	geonode/locale/al/LC_MESSAGES/djangojs.po
#	geonode/locale/am/LC_MESSAGES/django.po
#	geonode/locale/am/LC_MESSAGES/djangojs.po
#	geonode/locale/ar/LC_MESSAGES/django.mo
#	geonode/locale/ar/LC_MESSAGES/django.po
#	geonode/locale/ar/LC_MESSAGES/djangojs.po
#	geonode/locale/bg_BG/LC_MESSAGES/django.po
#	geonode/locale/bg_BG/LC_MESSAGES/djangojs.po
#	geonode/locale/bn/LC_MESSAGES/django.po
#	geonode/locale/bn/LC_MESSAGES/djangojs.po
#	geonode/locale/de/LC_MESSAGES/django.mo
#	geonode/locale/de/LC_MESSAGES/django.po
#	geonode/locale/de/LC_MESSAGES/djangojs.po
#	geonode/locale/el/LC_MESSAGES/django.mo
#	geonode/locale/el/LC_MESSAGES/django.po
#	geonode/locale/el/LC_MESSAGES/djangojs.po
#	geonode/locale/en/LC_MESSAGES/django.mo
#	geonode/locale/en/LC_MESSAGES/django.po
#	geonode/locale/en/LC_MESSAGES/djangojs.po
#	geonode/locale/es/LC_MESSAGES/django.mo
#	geonode/locale/es/LC_MESSAGES/django.po
#	geonode/locale/es/LC_MESSAGES/djangojs.po
#	geonode/locale/fa/LC_MESSAGES/django.po
#	geonode/locale/fa/LC_MESSAGES/djangojs.po
#	geonode/locale/fa_IR/LC_MESSAGES/django.po
#	geonode/locale/fa_IR/LC_MESSAGES/djangojs.po
#	geonode/locale/fi/LC_MESSAGES/django.po
#	geonode/locale/fi/LC_MESSAGES/djangojs.po
#	geonode/locale/fil/LC_MESSAGES/django.po
#	geonode/locale/fil/LC_MESSAGES/djangojs.po
#	geonode/locale/fr/LC_MESSAGES/django.mo
#	geonode/locale/fr/LC_MESSAGES/django.po
#	geonode/locale/fr/LC_MESSAGES/djangojs.po
#	geonode/locale/hu/LC_MESSAGES/django.po
#	geonode/locale/hu/LC_MESSAGES/djangojs.po
#	geonode/locale/id/LC_MESSAGES/django.po
#	geonode/locale/id/LC_MESSAGES/djangojs.po
#	geonode/locale/it/LC_MESSAGES/django.mo
#	geonode/locale/it/LC_MESSAGES/django.po
#	geonode/locale/it/LC_MESSAGES/djangojs.mo
#	geonode/locale/it/LC_MESSAGES/djangojs.po
#	geonode/locale/ja/LC_MESSAGES/django.po
#	geonode/locale/ja/LC_MESSAGES/djangojs.po
#	geonode/locale/ka/LC_MESSAGES/django.po
#	geonode/locale/ka/LC_MESSAGES/djangojs.po
#	geonode/locale/km/LC_MESSAGES/django.po
#	geonode/locale/km/LC_MESSAGES/djangojs.po
#	geonode/locale/ko/LC_MESSAGES/django.po
#	geonode/locale/ko/LC_MESSAGES/djangojs.po
#	geonode/locale/lt/LC_MESSAGES/django.po
#	geonode/locale/lt/LC_MESSAGES/djangojs.po
#	geonode/locale/ne/LC_MESSAGES/django.po
#	geonode/locale/ne/LC_MESSAGES/djangojs.po
#	geonode/locale/nl_NL/LC_MESSAGES/django.po
#	geonode/locale/nl_NL/LC_MESSAGES/djangojs.po
#	geonode/locale/no/LC_MESSAGES/django.po
#	geonode/locale/no/LC_MESSAGES/djangojs.po
#	geonode/locale/pl/LC_MESSAGES/django.mo
#	geonode/locale/pl/LC_MESSAGES/django.po
#	geonode/locale/pl/LC_MESSAGES/djangojs.po
#	geonode/locale/pt/LC_MESSAGES/django.mo
#	geonode/locale/pt/LC_MESSAGES/django.po
#	geonode/locale/pt/LC_MESSAGES/djangojs.po
#	geonode/locale/pt_BR/LC_MESSAGES/django.po
#	geonode/locale/pt_BR/LC_MESSAGES/djangojs.po
#	geonode/locale/ro/LC_MESSAGES/django.po
#	geonode/locale/ro/LC_MESSAGES/djangojs.po
#	geonode/locale/ru/LC_MESSAGES/django.po
#	geonode/locale/ru/LC_MESSAGES/djangojs.po
#	geonode/locale/si/LC_MESSAGES/django.po
#	geonode/locale/si/LC_MESSAGES/djangojs.po
#	geonode/locale/sk/LC_MESSAGES/django.po
#	geonode/locale/sk/LC_MESSAGES/djangojs.po
#	geonode/locale/sq/LC_MESSAGES/django.po
#	geonode/locale/sq/LC_MESSAGES/djangojs.po
#	geonode/locale/sv/LC_MESSAGES/django.po
#	geonode/locale/sv/LC_MESSAGES/djangojs.po
#	geonode/locale/sw/LC_MESSAGES/django.po
#	geonode/locale/sw/LC_MESSAGES/djangojs.po
#	geonode/locale/ta/LC_MESSAGES/django.po
#	geonode/locale/ta/LC_MESSAGES/djangojs.po
#	geonode/locale/th/LC_MESSAGES/django.po
#	geonode/locale/th/LC_MESSAGES/djangojs.po
#	geonode/locale/tl/LC_MESSAGES/django.po
#	geonode/locale/tl/LC_MESSAGES/djangojs.po
#	geonode/locale/uk/LC_MESSAGES/django.po
#	geonode/locale/uk/LC_MESSAGES/djangojs.po
#	geonode/locale/vi/LC_MESSAGES/django.po
#	geonode/locale/vi/LC_MESSAGES/djangojs.po
#	geonode/locale/zh/LC_MESSAGES/django.po
#	geonode/locale/zh/LC_MESSAGES/djangojs.po
afabiani pushed a commit that referenced this issue Apr 8, 2021
* - Fix the "Upload" models fields and correctly link it to the "GeoNode Layer"

* - Adding REST Interfaces for the "Upload" models

* - [WIP] Upload api test cases

* - [WIP] Hide non processed resources

* - [WIP] Management of the Importer Session during Resume

* - [WIP] Hide dirty resources to everyone / Make sure Upload delete cascade is invoked everytime

* - [WIP] Update the UploaderSession state everytime we reload the importer session

* - [WIP] Set Uploader Session state to INVALID if any error occurs

* - [WIP] Get rid of incomplete uploads on templates that are not related to layer uploads

* - [WIP] Exposing Upload resume, delete and import URLs via REST APIs

* update workflow and ui of incomplete upload list

* replace import_id with id for the request to the api

* improve link button behaviour

* - Adapt failing test cases to the new "dirty_state" filtering condition

* - [WIP] Do not throw an exception when creating the "resume_url"

* - [WIP] Move the "Layer Info" button to the upload list

* add pagination and remove modal on list of incomplete upload

* Revert " - Adapt failing test cases to the new "dirty_state" filtering condition"

This reverts commit bee78bd.

* - [WIP] Make the Upload return URLs "state" aware

* - [WIP] Handle upload POST via REST interface

* - [WIP] Make "resourcebase_api" use "get_visible_resources" more efficiently

 - General code optimization and test case fixing

* - [WIP] Do not delete the Upload associated Layer

* - [WIP] Adding Upload Test Cases

* - Test fixes

* - Test fixes

* - Adding Upload Selenium Live Tests

* add processed item to the upload table

* - [WIP] Adding "create_date" to the Upload session

* sort the table by created date

* - Test fixes

* - Updating translations

* - Test fixes

* - Test fixes

Co-authored-by: allyoucanmap <[email protected]>
(cherry picked from commit 14070b2)

# Conflicts:
#	geonode/api/tests.py
#	geonode/locale/af/LC_MESSAGES/django.po
#	geonode/locale/af/LC_MESSAGES/djangojs.po
#	geonode/locale/al/LC_MESSAGES/django.po
#	geonode/locale/al/LC_MESSAGES/djangojs.po
#	geonode/locale/am/LC_MESSAGES/django.po
#	geonode/locale/am/LC_MESSAGES/djangojs.po
#	geonode/locale/ar/LC_MESSAGES/django.mo
#	geonode/locale/ar/LC_MESSAGES/django.po
#	geonode/locale/ar/LC_MESSAGES/djangojs.po
#	geonode/locale/bg_BG/LC_MESSAGES/django.po
#	geonode/locale/bg_BG/LC_MESSAGES/djangojs.po
#	geonode/locale/bn/LC_MESSAGES/django.po
#	geonode/locale/bn/LC_MESSAGES/djangojs.po
#	geonode/locale/de/LC_MESSAGES/django.mo
#	geonode/locale/de/LC_MESSAGES/django.po
#	geonode/locale/de/LC_MESSAGES/djangojs.po
#	geonode/locale/el/LC_MESSAGES/django.mo
#	geonode/locale/el/LC_MESSAGES/django.po
#	geonode/locale/el/LC_MESSAGES/djangojs.po
#	geonode/locale/en/LC_MESSAGES/django.mo
#	geonode/locale/en/LC_MESSAGES/django.po
#	geonode/locale/en/LC_MESSAGES/djangojs.po
#	geonode/locale/es/LC_MESSAGES/django.mo
#	geonode/locale/es/LC_MESSAGES/django.po
#	geonode/locale/es/LC_MESSAGES/djangojs.po
#	geonode/locale/fa/LC_MESSAGES/django.po
#	geonode/locale/fa/LC_MESSAGES/djangojs.po
#	geonode/locale/fa_IR/LC_MESSAGES/django.po
#	geonode/locale/fa_IR/LC_MESSAGES/djangojs.po
#	geonode/locale/fi/LC_MESSAGES/django.po
#	geonode/locale/fi/LC_MESSAGES/djangojs.po
#	geonode/locale/fil/LC_MESSAGES/django.po
#	geonode/locale/fil/LC_MESSAGES/djangojs.po
#	geonode/locale/fr/LC_MESSAGES/django.mo
#	geonode/locale/fr/LC_MESSAGES/django.po
#	geonode/locale/fr/LC_MESSAGES/djangojs.po
#	geonode/locale/hu/LC_MESSAGES/django.po
#	geonode/locale/hu/LC_MESSAGES/djangojs.po
#	geonode/locale/id/LC_MESSAGES/django.po
#	geonode/locale/id/LC_MESSAGES/djangojs.po
#	geonode/locale/it/LC_MESSAGES/django.mo
#	geonode/locale/it/LC_MESSAGES/django.po
#	geonode/locale/it/LC_MESSAGES/djangojs.mo
#	geonode/locale/it/LC_MESSAGES/djangojs.po
#	geonode/locale/ja/LC_MESSAGES/django.po
#	geonode/locale/ja/LC_MESSAGES/djangojs.po
#	geonode/locale/ka/LC_MESSAGES/django.po
#	geonode/locale/ka/LC_MESSAGES/djangojs.po
#	geonode/locale/km/LC_MESSAGES/django.po
#	geonode/locale/km/LC_MESSAGES/djangojs.po
#	geonode/locale/ko/LC_MESSAGES/django.po
#	geonode/locale/ko/LC_MESSAGES/djangojs.po
#	geonode/locale/lt/LC_MESSAGES/django.po
#	geonode/locale/lt/LC_MESSAGES/djangojs.po
#	geonode/locale/ne/LC_MESSAGES/django.po
#	geonode/locale/ne/LC_MESSAGES/djangojs.po
#	geonode/locale/nl_NL/LC_MESSAGES/django.po
#	geonode/locale/nl_NL/LC_MESSAGES/djangojs.po
#	geonode/locale/no/LC_MESSAGES/django.po
#	geonode/locale/no/LC_MESSAGES/djangojs.po
#	geonode/locale/pl/LC_MESSAGES/django.mo
#	geonode/locale/pl/LC_MESSAGES/django.po
#	geonode/locale/pl/LC_MESSAGES/djangojs.po
#	geonode/locale/pt/LC_MESSAGES/django.mo
#	geonode/locale/pt/LC_MESSAGES/django.po
#	geonode/locale/pt/LC_MESSAGES/djangojs.po
#	geonode/locale/pt_BR/LC_MESSAGES/django.po
#	geonode/locale/pt_BR/LC_MESSAGES/djangojs.po
#	geonode/locale/ro/LC_MESSAGES/django.po
#	geonode/locale/ro/LC_MESSAGES/djangojs.po
#	geonode/locale/ru/LC_MESSAGES/django.po
#	geonode/locale/ru/LC_MESSAGES/djangojs.po
#	geonode/locale/si/LC_MESSAGES/django.po
#	geonode/locale/si/LC_MESSAGES/djangojs.po
#	geonode/locale/sk/LC_MESSAGES/django.po
#	geonode/locale/sk/LC_MESSAGES/djangojs.po
#	geonode/locale/sq/LC_MESSAGES/django.po
#	geonode/locale/sq/LC_MESSAGES/djangojs.po
#	geonode/locale/sv/LC_MESSAGES/django.po
#	geonode/locale/sv/LC_MESSAGES/djangojs.po
#	geonode/locale/sw/LC_MESSAGES/django.po
#	geonode/locale/sw/LC_MESSAGES/djangojs.po
#	geonode/locale/ta/LC_MESSAGES/django.po
#	geonode/locale/ta/LC_MESSAGES/djangojs.po
#	geonode/locale/th/LC_MESSAGES/django.po
#	geonode/locale/th/LC_MESSAGES/djangojs.po
#	geonode/locale/tl/LC_MESSAGES/django.po
#	geonode/locale/tl/LC_MESSAGES/djangojs.po
#	geonode/locale/uk/LC_MESSAGES/django.po
#	geonode/locale/uk/LC_MESSAGES/djangojs.po
#	geonode/locale/vi/LC_MESSAGES/django.po
#	geonode/locale/vi/LC_MESSAGES/djangojs.po
#	geonode/locale/zh/LC_MESSAGES/django.po
#	geonode/locale/zh/LC_MESSAGES/djangojs.po
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gnip A GeoNodeImprovementProcess Issue
Projects
None yet
Development

No branches or pull requests

5 participants