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

Model suggestion #75

Open
EllaQuimica opened this issue Dec 21, 2020 · 5 comments
Open

Model suggestion #75

EllaQuimica opened this issue Dec 21, 2020 · 5 comments

Comments

@EllaQuimica
Copy link
Collaborator

Hi Manuel,

This is our proposal for models, please have a look and let's know if it is ok.

Any feedback welcome

cc @Elena-GHub
imagen

@humitos
Copy link
Owner

humitos commented Dec 22, 2020

Hi! This is a good start!

Here are some notes:

  • model names usually are in singular and with first letter in uppercase (e.g. Tag)
  • there is no need to model User since it's a model that already comes with Django
  • can you describe what the model tags represents exactly?
  • tags.title could probably be renamed to be called name
  • why are you using char for pois.coordinates?
  • I don't think we will relate User <-> Poi directly, but through a Layer model
    • each User will have at least one Layer, called "Base Layer"
    • every Poi the user adds, will be added to the "Base Layer" by default
    • the User can create as many Layer as they want
    • when adding a new Poi if there are extra Layer for this User, they can select a different one than "Base Layer"

I suppose you could start by implementing these models you have here and it will be OK. I'm sure there are some changes/adjustments to be done while writing them, but I think we can discuss them more in deeply on the PR. I'm sure you will realize/fix/consider some of my notes (and others) while writing them.

@Elena-GHub
Copy link
Collaborator

Elena-GHub commented Dec 23, 2020

Thanks for the feedback!

Here are some notes:

* model names usually are in singular and with first letter in uppercase (e.g. `Tag`)

Sure! Sorry, those are the names of the tables. The models will follow that convention.

* there is no need to model `User` since it's a model that already comes with Django

Agreed, this was just to see it in our design.

* can you describe what the model `tags` represents exactly?

The tags are criteria by which you would like a poi to be displayed or not (like the checkboxes you have in the original website).

* `tags.title` could probably be renamed to be called `name`

No problem. We also considered it. We will leave it as tags.title. [edit] sorry, we meant tags.name

* why are you using `char` for `pois.coordinates`?

We did not know about the geometry data type, but your comment made us check it out and this is now changed to "geometry".

* I don't think we will relate `User <-> Poi` directly, but through a `Layer` model     
  * each `User` will have _at least_ one `Layer`, called "Base Layer"
  * every `Poi` the user adds, will be added to the "Base Layer" by default
  * the `User` can create as many `Layer` as they want
  * when adding a new `Poi` if there are extra `Layer` for this `User`, they can select a different one than "Base Layer"

We see the layer point but we are not sure of why this should be a model as opposed to a pivot table. Could you please explain why a model would be more appropriate?

@Elena-GHub
Copy link
Collaborator

Estuvimos pensando en tu sugerencia de "Layer" como modelo. Puesto que no lo acabamos de ver, pensamos que podría corresponder a esta estructura en la que usamos etiquetas personalizadas creadas por el usuario.
UPOI_DB_relationships_2

@humitos
Copy link
Owner

humitos commented Dec 24, 2020

Hola! 👋

No entiendo bien qué significa custom_tags en este gráfico. Creo que podría estar bien dependiendo cuál sea su significado 😅 De cualquier forma, creo que nos estamos anticipando a una funcionalidad que aún no existe en la web actual y podríamos dejar esta parte del modelado (Layer) para más adelante.

En este momento del proyecto, lo más importante creo que es avanzar con el modelo Poi y que escriban el código Python necesario, creen la migración, usen un poco el admin para crear/editar/borrar datos y demás.

Por otro lado, @EllaQuimica me comentó que estaban escribiendo tests para este model, pero de momento esto no es muy relevante y les aconsejo que no se centren en escribir estos tests. Podemos empezar a aplicar TDD una vez que tengamos funcionabilidad en la aplicación que podamos testear con casos concretos.

@Elena-GHub
Copy link
Collaborator

¡Hola, Manuel!
Siguiendo tus recomendaciones hemos trabajado en el modelo Poi y hemos creado el usuario Admin. Todo queda recogido en el PR#76.
Verás que hay un par de cosillas pendientes para las que nos gustaría contar con tus sabios consejos.
PS: hemos dejado los tests para más adelante

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