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

Allow to update a published map #1451

Closed
cmongut opened this issue Jan 8, 2020 · 7 comments
Closed

Allow to update a published map #1451

cmongut opened this issue Jan 8, 2020 · 7 comments
Assignees

Comments

@cmongut
Copy link
Contributor

cmongut commented Jan 8, 2020

As a user I want to update a published map so I don't generate a new one every time I call publish.

One approach that can work, after discussing with a data scientist, would be to use the same API we have for datasets.

map.publish('map_name', if_exists='replace', password=None)

Thoughts @andy-esch, @oleurud?

@simon-contreras-deel
Copy link
Contributor

simon-contreras-deel commented Jan 9, 2020

Right now, we have a publish and an update_publication methods:

  • publish: always creates a new custom visualization
  • update: update an existing one (without changing other things like api key for example)

You can see them in action (you can see how the update does not change the id):

image

So, going to the problem, if I understand it well, the point is:

  • by default, avoid creating a Kuviz if one already exists with the same name (it happens rerunning a notebook for example) returning an error
  • adding the possibility of updating it if already exists

@cmongut
Copy link
Contributor Author

cmongut commented Jan 9, 2020

Yeap, the idea is that publish can update an existing publication

@simon-contreras-deel
Copy link
Contributor

simon-contreras-deel commented Jan 9, 2020

Ok, so the solution could be the following one.

In any case:
CARTOframes:

  • add a new parameter if_exists in publish method with 2 options:
    • fail (default)
    • replace

cartodb

  • return an error when a user tries to create/update a kuviz with a name already used by herself. I see 2 possible solutions:
    • adding a partial unique index: this would be my option if we were from scratch, but now, we are going to have several cases in violation of that index.
    • ensuring it by code: we will need to make a previous query when creating and updating a kuviz to ensure it

Option 1:

Check from CARTOframes if already exists a Kuviz with that name:

carto-python:

  • create a get Kuviz by name

CARTOframes:

  • use get Kuviz by name in publish

cartodb

  • The best option here would be using the partial index

Option 2:

Do it in backend

CARTOframes:

  • send the if_exists value to backend in publish

cartodb

  • the best option would be using code validation

The second one seems easier (not better)

@simon-contreras-deel
Copy link
Contributor

We are going to go with the option 2 (we already have duplicated, several duplicated).

Another important thing or pain: a user has several kuviz with the same name and tries to update one of them without changing the name. In this case, we have 2 options:

  • fail: name already in use (even if the user is not updating the name)
  • work (we would need to add a very ugly condition in backend).

My vote is for fail.

In any case, if the user updates the name to an existing one, it will fail.

@simon-contreras-deel
Copy link
Contributor

simon-contreras-deel commented Jan 10, 2020

Since we are not going to change data from clients, the option of adding a unique index (partial index: when type is Kuviz) is not feasible. We could add it taking into account the date (the partial index would be type = 'kuviz' and created_at > '2020-01-10'::timestamp), but in any case, we should still check for duplicates by code and thinking about it as an array of possible values. So, my idea is to ensure uniqueness only by code

Another interesting point: if a user has several kuviz with the name pepito and she tries to publish a new one using pepito as name and replace as if_exists, we have 2 options:

  • fail: we can not ensure what kuviz should be updated
  • clean it: drop all the previous one and create a new one

My vote is for the clean it since the user is not worried about the previous Kuviz or Kuvizs.

@Jesus89 Jesus89 removed this from the [1.0rc1] Feedback enhancements milestone Jan 10, 2020
@cmongut
Copy link
Contributor Author

cmongut commented Jan 10, 2020

👍

We should clean them because the user has said to replace them explicitly.

@simon-contreras-deel
Copy link
Contributor

Validating it in the acceptance, we have a change :(

When we said: clean it: drop all the previous one and create a new one, it is not the required behavior. Instead of that, we want to reuse the "previous" Kuviz. And what is the previous one taking into account that we could have several with the same name? the last one created with that name.

Being more user-faced:

  • a user run a notebook which calls to publish
  • the user rerun the notebook

The second publish should reuse the same kuviz as the first run.

@Jesus89 Jesus89 closed this as completed Jan 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants