Skip to content

Resuelto: Acción delete funciona correctamente#4

Open
nmarmon wants to merge 1 commit intojonabasque:masterfrom
nmarmon:bug#2
Open

Resuelto: Acción delete funciona correctamente#4
nmarmon wants to merge 1 commit intojonabasque:masterfrom
nmarmon:bug#2

Conversation

@nmarmon
Copy link

@nmarmon nmarmon commented Jan 7, 2015

El controlador Csong.php y el modelo específico Msong.php no funcionaban porque no coincidían el número de parámetros entre la llamada y la declaración del métodos deleteSong(). Había un parámetro llamado $entity que en principio no tiene utilidad, así que se ha eliminado.

El método Delete del CRUD del modelo principal no estaba bien, ha habido que corregir algunas líneas para que algunos índices y nombres de variables se ejecutasen correctamente, de tal forma que cuando se ejecuta el $query->execute(), el registro es eliminado de la tabla "song".

En estos momento, el método delete del módulo principal elimina un registro seleccionado reviamente por su identificador.

@jonabasque
Copy link
Owner

No hay por que eliminar la opción de pasarle un parámetro $entity para decirle true o false y así con este eliminar la tabla. Es decir, el método delete, que puede eliminar? registro, tabla e incluso bbdd si nos ponemos.

Primer parámetro $table que siempre tiene que recibir un string con la tabla del modelo del módulo (hay que decidir más adelante si puede un módulo trabajar con dos tablas, que yo creo que no, y si usar a otro). En $params tiene que venir el $id y en $entity si no viene nada no pasa nada, es false, se hace una validación y listo.

No puedes cambiar $params por $id en el delete del modelo general, siempre es $params y ha de venir como array desde la vista (quizás ahí estaba el error).

No puedo aceptarlo, no has arreglado el bug si no que has eliminado funcionalidad para que no haya bug, que no es lo mismo.

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

Successfully merging this pull request may close these issues.

2 participants