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

Feature/share experiments #66

Merged
merged 9 commits into from
Feb 11, 2025
Merged

Feature/share experiments #66

merged 9 commits into from
Feb 11, 2025

Conversation

juanNH
Copy link
Contributor

@juanNH juanNH commented Feb 8, 2025

No description provided.

)

experiment.shared_institutions.remove(institution)
experiment.save()
Copy link
Member

Choose a reason for hiding this comment

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

Cuando se hacen cambios en relaciones ManyToMany como en la línea de arriba, no es necesario hacer un save(). Este métodos solo debe llamarse cuando se edita algún campo particular, y se debe llamar solo sobre el modelo cambiado:

experiment.name = 'X'
experiment.save()

# O...
experiment.user.apellido = 'Sanchez'
experiment.user.save()  # Fijate que el save() solo es en el objeto cambiado

Por ende, eliminar esta línea

)

experiment.shared_users.remove(user)
experiment.save()
Copy link
Member

Choose a reason for hiding this comment

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

Idem comentario anterior

)

experiment.is_public = not experiment.is_public
experiment.save()
Copy link
Member

Choose a reason for hiding this comment

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

Para optimizar la performance del guardado, se puede pasar un parámetro llamado update_fields que hace que el UPDATE que ejecuta el ORM por atrás solo incluya ese campo. Cambiar a experiment.save(update_fields=['is_public'])

experiment = get_object_or_404(Experiment, id=experiment_id)

associated_user_ids = experiment.shared_users.values_list('id', flat=True)
print(associated_user_ids)
Copy link
Member

Choose a reason for hiding this comment

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

Eliminar print()!

institution = get_object_or_404(Institution, id=institution_id)

experiment.shared_institutions.add(institution)
experiment.save()
Copy link
Member

Choose a reason for hiding this comment

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

Idem comentarios anteriores

experiment.save()

serializer = InstitutionSerializer(institution)
return Response(serializer.data, status=status.HTTP_200_OK)
Copy link
Member

Choose a reason for hiding this comment

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

Sacar el status acá ya que por defecto es el 200

user = get_object_or_404(User, id=user_id)

experiment.shared_users.add(user)
experiment.save()
Copy link
Member

Choose a reason for hiding this comment

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

Idem comentarios anteriores

@@ -92,6 +92,10 @@ export class InstitutionsPanel extends React.Component<{}, InstitutionsPanelStat
}
}

/**
* Default modal.
* @returns {ConfirmModal}
Copy link
Member

Choose a reason for hiding this comment

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

Ojo los warnings de ESLint, correr npm run check-lint o npm run check-all para ver todos los warnings

) :
(
<Icon
title='If this is checked all the users in the platform can see (but not edit or remove) this element'
Copy link
Member

Choose a reason for hiding this comment

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

Ojo, quedó el mismo title que arriba!

@Genarito Genarito merged commit d881dae into v5.0.0 Feb 11, 2025
1 check passed
@Genarito Genarito deleted the feature/share_experiments branch February 11, 2025 18:32
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