humitos / osm-pois

Show POIs from OSM in a map using OverpassAPI
http://upoi.org/
GNU General Public License v2.0
22 stars 21 forks source link

Model suggestion #75

Open EllaQuimica opened 3 years ago

EllaQuimica commented 3 years ago

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 commented 3 years ago

Hi! This is a good start!

Here are some notes:

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Hola! :wave:

No entiendo bien qué significa custom_tags en este gráfico. Creo que podría estar bien dependiendo cuál sea su significado :sweat_smile: 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 commented 3 years ago

¡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