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

New cards #69

Merged
merged 19 commits into from
Mar 25, 2021
Merged

New cards #69

merged 19 commits into from
Mar 25, 2021

Conversation

ricardo8990
Copy link

@ricardo8990 ricardo8990 commented Mar 22, 2021

  • Agrega un campo al modelo base de to_dict llamado url_reference, con este campo podemos decidir si queremos que un objeto embebido tenga una URL como referencia o el objeto tal cual.
  • Corregí algunos tests que no estaban prob ando correctamente los objetos embebidos

Solo falta esperar a que se mezcle cuenca-validations para actualizar requirements.txt y setup.py

@ricardo8990 ricardo8990 self-assigned this Mar 22, 2021
@ricardo8990 ricardo8990 added the enhancement New feature or request label Mar 22, 2021
@codecov
Copy link

codecov bot commented Mar 22, 2021

Codecov Report

Merging #69 (a4da138) into main (ca227d0) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              main       #69   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           10        10           
  Lines          258       258           
  Branches        42        42           
=========================================
  Hits           258       258           
Flag Coverage Δ
unittests 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca227d0...a4da138. Read the comment docs.

Ricardo Sánchez added 2 commits March 22, 2021 21:52
Copy link
Member

@alexviquez alexviquez left a comment

Choose a reason for hiding this comment

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

Porque no mejor usas un header para definir si se va a consultar el contenido completo o solo se va a consultar el objeto principal, más los url, me refiero a algo como:

headers = {"Content-Attributes":"uris"}
# ó
headers = {"Content-Attributes":"resources"}

eso haría que en otras librerías con el hecho de agregar ese header podamos descargar los objetos anidados.

@alexviquez
Copy link
Member

y quitar los prints

@ricardo8990
Copy link
Author

Porque no mejor usas un header para definir si se va a consultar el contenido completo o solo se va a consultar el objeto principal, más los url, me refiero a algo como:

headers = {"Content-Attributes":"uris"}
# ó
headers = {"Content-Attributes":"resources"}

eso haría que en otras librerías con el hecho de agregar ese header podamos descargar los objetos anidados.

Creo que si es algo que podríamos agregar, aunque este caso sería el primer paso para llegar a ello. Ya que sería durante los blueprints que de acuerdo al tipo de header se debería de mandar el parámetro o no. En este caso agregué el parámetro ya que los CardActivation de Cuenca siempre deben incluir el objeto embebido, por otro lado, no sé si eso haría que se tengan que estar editando los headers para diferentes peticiones, por ejemplo, alguien usando cuenca-python tendría que hacer algo como:

import cuenca

# Poner headers como resources
cuenca.http.session.headers['Content-Attributes'] = 'resources'
card_activation = CardActivation.create(...)
card = card_activation.card

# Para obtener ahora sus card_transactions, regresar los headers como estaban
cuenca.http.session.headers['Content-Attributes'] = 'uris'
card_activation = CardTransactions.all()

@alexviquez
Copy link
Member

Porque no mejor usas un header para definir si se va a consultar el contenido completo o solo se va a consultar el objeto principal, más los url, me refiero a algo como:

headers = {"Content-Attributes":"uris"}
# ó
headers = {"Content-Attributes":"resources"}

eso haría que en otras librerías con el hecho de agregar ese header podamos descargar los objetos anidados.

Creo que si es algo que podríamos agregar, aunque este caso sería el primer paso para llegar a ello. Ya que sería durante los blueprints que de acuerdo al tipo de header se debería de mandar el parámetro o no. En este caso agregué el parámetro ya que los CardActivation de Cuenca siempre deben incluir el objeto embebido, por otro lado, no sé si eso haría que se tengan que estar editando los headers para diferentes peticiones, por ejemplo, alguien usando cuenca-python tendría que hacer algo como:

import cuenca

# Poner headers como resources
cuenca.http.session.headers['Content-Attributes'] = 'resources'
card_activation = CardActivation.create(...)
card = card_activation.card

# Para obtener ahora sus card_transactions, regresar los headers como estaban
cuenca.http.session.headers['Content-Attributes'] = 'uris'
card_activation = CardTransactions.all()

pero podrías hacerlo desde el configure no?, hay recursos que en realidad no harían nada distinto como card_transactions ya que en el to_dict de mongo no tiene campos que tengan referencias, pero del lado del cliente como cuenca-dart o cuenca-node en un futuro sería valioso en un solo request obtener el contexto completo de un balance-entrie o así

@ricardo8990
Copy link
Author

Porque no mejor usas un header para definir si se va a consultar el contenido completo o solo se va a consultar el objeto principal, más los url, me refiero a algo como:

headers = {"Content-Attributes":"uris"}
# ó
headers = {"Content-Attributes":"resources"}

eso haría que en otras librerías con el hecho de agregar ese header podamos descargar los objetos anidados.

Creo que si es algo que podríamos agregar, aunque este caso sería el primer paso para llegar a ello. Ya que sería durante los blueprints que de acuerdo al tipo de header se debería de mandar el parámetro o no. En este caso agregué el parámetro ya que los CardActivation de Cuenca siempre deben incluir el objeto embebido, por otro lado, no sé si eso haría que se tengan que estar editando los headers para diferentes peticiones, por ejemplo, alguien usando cuenca-python tendría que hacer algo como:

import cuenca

# Poner headers como resources
cuenca.http.session.headers['Content-Attributes'] = 'resources'
card_activation = CardActivation.create(...)
card = card_activation.card

# Para obtener ahora sus card_transactions, regresar los headers como estaban
cuenca.http.session.headers['Content-Attributes'] = 'uris'
card_activation = CardTransactions.all()

pero podrías hacerlo desde el configure no?, hay recursos que en realidad no harían nada distinto como card_transactions ya que en el to_dict de mongo no tiene campos que tengan referencias, pero del lado del cliente como cuenca-dart o cuenca-node en un futuro sería valioso en un solo request obtener el contexto completo de un balance-entrie o así

Regresé los archivos a cómo estaban, solo dejé la corrección del test que realmente no estaba probando algo en concreto.

alexviquez
alexviquez previously approved these changes Mar 24, 2021
Copy link
Member

@alexviquez alexviquez left a comment

Choose a reason for hiding this comment

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

ajustar las librerías y crear un issue para explorar el tema que platiccamos de los headers a futuro, incluso se me ocurre que podríamos no manejarlo del header, si no manejarlo desde la url tal y como jsonapi lo hace, es decir en el get añadir un attributes=related_transaction, funding_instrument y si no trae como atributo el campo, no hacer el fetch y dejar solo la url del recurso, en caso contrario crear el objeto anidado

@ricardo8990
Copy link
Author

ajustar las librerías y crear un issue para explorar el tema que platiccamos de los headers a futuro, incluso se me ocurre que podríamos no manejarlo del header, si no manejarlo desde la url tal y como jsonapi lo hace, es decir en el get añadir un attributes=related_transaction, funding_instrument y si no trae como atributo el campo, no hacer el fetch y dejar solo la url del recurso, en caso contrario crear el objeto anidado

Issue #72 y librerías actualizadas

Copy link
Member

@alexviquez alexviquez left a comment

Choose a reason for hiding this comment

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

fierro

@ricardo8990 ricardo8990 merged commit a2359b1 into main Mar 25, 2021
@ricardo8990 ricardo8990 deleted the new_cards branch March 25, 2021 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants