Feature/share biomarker #76
Conversation
| @@ -418,7 +429,7 @@ def get_queryset(self): | |||
|
|
|||
| class UsersExperimentsSharedListView(generics.ListAPIView): | |||
| """ | |||
| REST endpoint: Get all users associated with a specific experiment. | |||
| REST endpoint: Get all institution associated with a specific experiment. | |||
There was a problem hiding this comment.
Estaba bien el comentario anterior! Terminás devolviendo una QuerySet de Users, así que acá debería volver a decir "users"
src/api_service/views.py
Outdated
| try: | ||
| experiment: Experiment = Experiment.objects.get( | ||
| pk=experiment_id, user=self.request.user) | ||
| #Todo: modificar Query |
There was a problem hiding this comment.
- El filter devuelve muchos resultados, para obtener uno sol se puede simplemente hacer un
.geten vez de.filter. - Eliminar el
.distinct().get()ya que ahora es solo un resultado - Eliminar ese comentario con el TODO
src/biomarkers/serializers.py
Outdated
| @@ -57,6 +58,11 @@ class Meta: | |||
| model = MethylationIdentifier | |||
| exclude = ['biomarker'] | |||
|
|
|||
| class LimitedUserSerializer(serializers.ModelSerializer): | |||
There was a problem hiding this comment.
Este serializer ya existe en api_service/serializers.py importar ese!
| @@ -70,7 +77,17 @@ class BiomarkerDetail(generics.RetrieveUpdateDestroyAPIView): | |||
| """REST endpoint: get, modify and delete for Biomarker model.""" | |||
|
|
|||
| def get_queryset(self): | |||
| return Biomarker.objects.filter(user=self.request.user) | |||
| user = self.request.user | |||
There was a problem hiding this comment.
Si solo lo estás haciendo para hacer un retrieve en el frontend chocando contra una URL del estilo biomarkers/<pk>/ no hace falta agarrar la PK y filtrar por eso en la vista. Alcanza con devolver un QuerySet (filtrando por is_public, y todos los filtros que pusiste, excepto por la PK). Y listo! Django REST Framework va a filtrar después por la PK automáticamente para obtener un solo resultado.
- Borrar filtro por PK
- Sacar el distinct
src/biomarkers/views.py
Outdated
|
|
||
| biomarker.is_public = not biomarker.is_public | ||
| biomarker.save() |
There was a problem hiding this comment.
Optimizar poniendo como parámetro el update_fields con una lista con el nombre del único campo que se está actualizando (en este caso is_public)
| isAdding: boolean, | ||
| user: { id: number, username: string } | ||
| } | ||
| interface Props extends SharedInstitutionsBiomarkerProps { |
There was a problem hiding this comment.
En lo posible siempre poner el nombre del componente del cual son las props! Si no vamos a estar lleno de interfaces llamadas "Props"! De paso, documentar los componentes de este archivo indicando qué es lo que renderizan
| isAdding: boolean, | ||
| user: { id: number, username: string } | ||
| } | ||
| interface Props extends SharedUsersBiomarkerProps { |
There was a problem hiding this comment.
Combinar ambos, quedaron dos interfaces separadas y solo se utiliza Props. Dejar el nombre SharedUsersBiomarkerProps con todos los campos. No exportar nada que no se use en otro archivo, así no agregamos ruido al proyecto!
| handleChangeConfirmModalState: (setOption: boolean, headerText: string, contentText: string, onConfirm: () => void) => void, | ||
| } | ||
|
|
||
| export const SharedUsersBiomarker = (props: Props) => { |
| @@ -88,7 +88,12 @@ interface BiomarkerSimple { | |||
| origin: BiomarkerOrigin, | |||
| state: BiomarkerState, | |||
| contains_nan_values: boolean, | |||
| column_used_as_index: string | |||
| column_used_as_index: string, | |||
| user: { | |||
There was a problem hiding this comment.
Entré a buscar a ver si existía la interface y se llama DjangoUserSimple. Hay un montón de ocurrencias en el proyecto del tipo { id: number, username: string }. Reemplazar todas por DjangoUserSimple
| ky.get(editUrl, { headers: myHeaders, signal: abortController.current.signal }).then((response) => { | ||
| response.json().then((jsonResponse: { id: number, username: string }[]) => { | ||
| console.log(jsonResponse) |
| isOpen: boolean, | ||
| institutions: { id: number, name: string }[], | ||
| biomarkerId: number, | ||
| isAdding: boolean, | ||
| user: { id: number, username: string } | ||
| } | ||
| interface Props extends SharedInstitutionsBiomarkerProps { | ||
| interface SharedInstitutionsBiomarkerProps extends SharedInstitutionsBiomarkerPropsExtend { |
There was a problem hiding this comment.
No entiendo por qué estás extendiendo de la prop de arriba, no es más fácil dejar una sola unificada? Además se está exportando, pero no veo que se use en ningún otro lado en el PR
| isOpen: boolean, | ||
| users: { id: number, name: string }[], | ||
| biomarkerId: number, | ||
| isAdding: boolean, | ||
| user: { id: number, username: string } | ||
| } | ||
| interface Props extends SharedUsersBiomarkerProps { | ||
| interface SharedUsersBiomarkerProps extends SharedUsersBiomarkerPropsExtend { |
| export const SharedUsersBiomarker = (props: Props) => { | ||
| /** | ||
| * Modal that shows user shared to a biomarker | ||
| * @param {SharedUsersBiomarkerProps} props Component props. |
There was a problem hiding this comment.
Evitar estas anotaciones, esto es tipado para JS, nosotros usamos Typescript, así que no hace falta duplicar todos los tipos en la documentación!
No description provided.