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

Creating models based on the design developed last week on steps and flow #75

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

cerdas16
Copy link

No description provided.

Copy link
Collaborator

@luisza luisza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Muy bien, creo que los modelos son suficientes, puede que durante la implemantación ocupe otros campos, pero inicialmente está bien.

djgentelella/models.py Outdated Show resolved Hide resolved
djgentelella/models.py Outdated Show resolved Hide resolved
djgentelella/models.py Outdated Show resolved Hide resolved
djgentelella/models.py Outdated Show resolved Hide resolved
… steps temporarily and storage in DB (details are missing, such as django actions, the correct creation of actions, skipconditions and states needs to be reviewed), in addition, there are certain changes in the models and shapes, small details


def save_steps(steps_data_json):
from djgentelella.forms.forms import GTStepForm
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Esta importación del formulario GTStepForm no debe ir aquí.

rows: 1
}).run();

console.log(stepData);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eliminar todos los prints o console que hayas subido.

pre_action = models.ManyToManyField(GTActionsStep, related_name='pre_steps', verbose_name=_("Pre Action"))
status_id = models.ForeignKey(GTStatus, on_delete=models.CASCADE, blank=True, null=True, verbose_name=_("Status"))
form = models.ManyToManyField(GTDbForm, related_name='forms', verbose_name=_("Form"), null=True)
post_action = models.ManyToManyField(GTActionsStep, related_name='post_steps', verbose_name=_("Post Action"), null=True)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Los campos de tipo ManyToManyField no necesitan el null=True.

Copy link
Collaborator

@marcelagz marcelagz Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Falta la migración de estos modelos.

Además el dato blank=False no es necesario definirlo en modelos, adjunto los valores por defecto de la herencia de la clase Field que trae django.

imagen


<script>

var stepData = JSON.parse(sessionStorage.getItem('stepData')) || {};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revisando me di cuenta que al agregar pasos a un flujo y luego darle atrás a la página del navegador regresaba al formulario de creación del flujo le cambie el nombre y al guardar podía visualizar los pasos que agregue al flujo anterior, esto quiere decir que la variable stepData no se esta limpiando.

Es necesario una alerta de confirmación para el usuario en caso de que el mismo por error o porque si salga de la página donde esta editando el flujo y la pregunta de confirmación requiere preguntar si desea guardar los cambios realizados y si indica que no se perderá el progreso y seguidamente reiniciar la variable stepData.

if form.is_valid():
form.save()
return redirect('flow')
else:
Copy link
Collaborator

@marcelagz marcelagz Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Este else no es tan necesario puedes declarar el formulario apenas inicias la función para que quede por defecto, lo mismo para status y condition.

def save_steps(steps_data_json):
from djgentelella.forms.forms import GTStepForm
for step in steps_data_json:
step_data = step['data']
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Como aseguras que esa clave data exista? Desde el navegador podemos modificar la data que enviamos al post de la función create_flow haciendo que la función el sistema caiga con errores, debemos validar ese campo stepsData ya sea con un formulario o con un serializador porque ahorita mismo resulta fácil sabotear la data del post al no haber validación de ello.

save_actions(step_data, step_instance)
print('exito')
else:
print(form.errors)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eliminar prints

print(form.errors)

def save_actions(step_data, step_instance):
from djgentelella.forms.forms import GTActionsStepForm
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Esta importación no debe ir acá.

</form>
</div>
</div>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hay un div que sobra aquí.


def step_index(request, id):

step = GTStep()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Estas creando la instancia de un paso(GTStep) pero no estas logrando hacer nada con ella, porque en el código JS modificas los valores de los campos con lo exista en la sesión. Sino vas a guardar la instancia o hacer algo con ella entonces es mejor no pasarla.


class RedirectButtonWidget(Widget):
def render(self, name, value, attrs=None, renderer=None):
action_url = reverse('actions')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hay que mejorar la definción de esta url porque es demasiado general, recomiendo utilizar "flow_actions" o algo similar.

Todas las urls de flujo pueden partir de "flow" por ejemplo:

path("flow", ...., name="index_flow"),
path("flow/create", ...., name="create_flow")
path("flow/actions", ...., name="actions_flow")

De esta manera separamos todas las urls relacionadas a flow y no chocamos con otras urls a futuro, porque sino a futuro nos vemos obligados a actualizarlas y revisar donde son llamadas y por ende nos quitaría tiempo de algo que podemos mejorar ahorita 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.

3 participants