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

Uniformation of Edit Properties and detail card #5772

Closed
1 of 10 tasks
offtherailz opened this issue Aug 12, 2020 · 10 comments
Closed
1 of 10 tasks

Uniformation of Edit Properties and detail card #5772

offtherailz opened this issue Aug 12, 2020 · 10 comments

Comments

@offtherailz
Copy link
Member

offtherailz commented Aug 12, 2020

Description

Now the Edit Properties tool (from home page) and the Save tools (From map/dashboard and stories) use 2 different implementations:

  • One is valid for internal Save tool of Map, Dashboard and stories and use the generic ResourceModal component, self contained
  • One is valid only for Map and FeaturedMap in the home page. They use specific epics and checks, and have also the map details dialog

We need to reuse the generic component also for the Map/FeaturedMaps. This means that:

Acceptance Criteria

  • The generic resource modal component also have detail card.
  • An option should allow/deny to see/hide it by user role.
  • The detail card should be optional, because dashboards, for the moment, must not have it.
  • Detail card should be visible in Map's Save or Save As tool. <-- this can be tested from UI.

What kind of improvement you want to add? (check one with "x", remove the others)

  • Minor changes to existing features
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

Note for Developer

  • The ResourceModal support auto save of linkedResources and attributes, so please keep the same self-contained approch applying your changes, adding a fragment and reusing the same implicit save system you find for other attributes, linked resources (thumbnail) and permissions. At the end, remove all the logic that is not used anymore.

  • After this task has been done, the pull request Responsive Design - Info Box #5396 can continue, so the entry point for the administrator for the detail card is the Save / Save as tool.

  • The origina 'epic' for this task was Related resources saving flow issues due to permission updates - Reuse dashboard code #2908, but it's old, so some information can not be true anymore, I add it just as reference.

Notes for testers

Many issues was related to the tool that is going to be replaced with an existing one. So after this issue is closed, you can (and also developer should check) see that the following issues has been solved.

Now I'm closing them in favour of this one.


#3096 _Steps to reproduce_

Create a new map, with a thumbnail and detail card
Edit the map you created removing the thumbnail, then save again
Edit again the map and try to edit detail
Try to Save
Expected Result

The map is saved again with changes to the detail card
Current Result

Click on save button has no effect. You have to edit also title or description to make it work.


#3406

This should have been solved by fixing the problem in NODATA handling (#5760) plus this fix.

Steps to reproduce
MAPS
Login as admin
Create new map
Save writing only the name
Go back to homepage
From homepage, set a thumbnail, some detials and the permission Testers can edit
Logout from admin and login as tester
Open edit properties from homepage
Problem 1: Can't see the details (NO_DETAILS_AVAILABLE) and can't edit them
Problem 2: If I set a new thumbnail, the prevous one is not deleted (DELETE 403 forbidden)

DASHBOARDS AND STORIES
Login as admin
Create new dashboard/story
Save writing only the name
Go back to homepage
From homepage, set a thumbnail
Problem 1: The console gives the following error: GET .../thumbnail 404 (Not Found)

Login as admin
Create new dashboard/story
Save writing only the name
Go back to homepage
From homepage, set the permission Testers can edit
Logout from admin and login as tester
Open edit properties from homepage
Problem 2: Tryng to set a thumbnail in console: GET .../thumbnail 404 Not Found


- #4440 Improvement: allow editing detail card from Map viewer.
#4443

Steps to reproduce

  • click on new map
  • click on save
  • add details from edit properties
  • add everyone can view from edit properties
  • click on save
  • logout
  • click show map details

Expected Result
The details are visible in the window

Current Result
The details windows shows the error: No_details_available

@vlt1
Copy link
Contributor

vlt1 commented Aug 12, 2020

After this, the pull request #5396 can continue, so the entry point for the administrator for the detail card is the Save / Save as tool

@offtherailz you want first for this task to be done, merged to master and then merge master to #5396 and continue work on #5396?

offtherailz pushed a commit that referenced this issue Sep 10, 2020
* Adopt general SaveModal to MapGrid

* added translations

* post review rework

* removed commented out code

* added tests
@ElenaGallo
Copy link
Contributor

ElenaGallo commented Sep 15, 2020

@vlt1 here are some notes after testing:

  • Updating a map description after adding a detail causes an issue

How to Reproduce

  • Open a map
  • On Burger Menu click on Save as
  • Add title and detail
  • Save
  • On Burger Menu click on Save
  • Add a description

Current Result
save1

Uder usefull information
The same problem occurs when opening the Edit properties from the homepage and adding the description

  • The DASHBOARDS AND STORIES issues that are described here have not been solved

@ElenaGallo ElenaGallo self-assigned this Sep 15, 2020
@offtherailz
Copy link
Member Author

offtherailz commented Sep 22, 2020

@ElenaGallo I linked to this issue also the #5861 that introduces changes to the detail card. Please test them together with the issues you notified, because Updating a map description after adding a detail causes an issue seems to be solved, didn't tested the second point.
If so, we coulld re-open only the related issue #3406

@ElenaGallo
Copy link
Contributor

The following problem are present on console:

  • Login as admin
  • Create new map/dashboard/story
  • Save writing only the name
  • Go back to homepage
  • From homepage, set a thumbnail

Problem 1: The console gives the following error: GET .../thumbnail 404 (Not Found) and GET .../details 404 (Not Found)

  • Login as admin
  • Create new map/dashboard/story
  • Save writing only the name
  • Go back to homepage
  • From homepage, set permissions (or details only form map)

Problem 2: The console gives the following error: GET .../details 404 (Not Found)

  • Login as admin
  • Create new dashboard/story
  • Save writing only the name
  • Go back to homepage
  • From homepage, set the permission Testers can edit
  • Logout from admin and login as tester

Problem 3: The console gives the following errors:

  • https://dev.mapstore2.geo-solutions.it/mapstore/rest/geostore/extjs/resource/29362 403 (Not Found)
  • https://dev.mapstore2.geo-solutions.it/mapstore/rest/geostore/resources/resource/29362/attributes 404 (Not Found)
  • https://dev.mapstore2.geo-solutions.it/mapstore/rest/geostore/data/29362 403 (Not Found)
  • Login as admin
  • Create new dashboard/story
  • Save writing only the name
  • Go back to homepage
  • From homepage, set the permission Testers can edit
  • Logout from admin and login as tester
  • Open edit properties from homepage
  • Set a thumbnail

Problem 4: The console gives the following errors: GET .../thumbnail 404 (Not Found) and GET .../details 404 (Not Found)

  • Login as admin
  • Create new map
  • Save writing only the name
  • Go back to homepage
  • From homepage, set the permission Testers can edit
  • Logout from admin and login as tester
  • Open edit properties from homepage
  • Set a thumbnail

Problem 5: The console gives the following errors:

  • GET .../permissions 403 (Not Found)
  • GET .../thumbnail 404 (Not Found)
  • GET .../details 404 (Not Found)

@tdipisa tdipisa closed this as completed Sep 24, 2020
@tdipisa tdipisa reopened this Sep 24, 2020
@tdipisa
Copy link
Member

tdipisa commented Sep 24, 2020

@offtherailz the image plugin included in the text editor within this issue needs to be reviewed. Let's talk about that.

@vlt1
Copy link
Contributor

vlt1 commented Oct 2, 2020

@ElenaGallo These are not errors really, just failed requests. There is, for example, logic in mapstore to check if there is data accessible of the attribute in order to determine whether to create a new one or update the old one. So these errors are not indicative of anything wrong.

@ElenaGallo
Copy link
Contributor

@vlt1 yes, i know. They are just failed requests, but should be removed if possible.

@tdipisa
Copy link
Member

tdipisa commented Oct 5, 2020

@vlt1 I agree with @ElenaGallo about this. There shouldn't be failing request if possible. @offtherailz @vlt1 are the performing of these failing requests needed? I've some concern on this to be honest.

offtherailz added a commit to offtherailz/MapStore2 that referenced this issue Oct 5, 2020
@offtherailz
Copy link
Member Author

About the details request, I see that detail card is always persisted, empty, on the first re-save.
I think this is not properly the correct workflow, even if it doesn't have consequences. This can be avoided by avoiding to pass details as linked resource if it is not defined.

I looked at it and the other errors are "correct" with the current workflow.

When try to save thumbnail or details, it checks first if the attribute exists, in order to guess if you have to replace the existing linked resource or create a new one.
This is needed to avoid to prevent the un-link of some previous resources, just before saving the resource.

Anyway, I think we could easely prevent this request error by doing a request to the whole resource attributes list of the map instead of the single value.
@vlt1 could we use at line 117 of geostore.js a request to the generic attribute list (that never fails) instad of the single value? This may avoid to have a newtwork error everytime we try to save (having network errors is always disorienting, so we'd like to avoid them, if possible).

@vlt1
Copy link
Contributor

vlt1 commented Oct 5, 2020

@offtherailz well there is getResourceAttributes request that gets the attribute list, so if it doesn't fail then yes that should work

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

No branches or pull requests

4 participants