mail notification to origin and destination when transfer is done #247
Conversation
| #byebug | ||
| # mail notificación pago | ||
| PaymentNotifier.transfer_source(@account.accountable.display_name_with_uid,transfer.amount.to_f/3600).deliver_now | ||
| PaymentNotifier.transfer_destination(@source.accountable.display_name_with_uid,transfer.amount.to_f/3600).deliver_now |
There was a problem hiding this comment.
Probablemente deberiamos enviar los correos desde una callback del modelo Transfer, Mirate after_commit, puede ser incluso un observer.
There was a problem hiding this comment.
No, please after_commit no 🙏
There was a problem hiding this comment.
Eso acolparia mucho el modelo con la notificación y lo pagaríamos caro
There was a problem hiding this comment.
Hagamos un service pues. Algo por el estilo de lo que hacemos en Katuma @sseerrggii
There was a problem hiding this comment.
Guay, me pondré a ello cuando te tenga al lado @enricostano 😉
| hours, minutes = minutes.divmod(60) | ||
| raw format("%s%d:%02d", ("-" if sign < 0), hours, minutes) | ||
| else | ||
| mdash |
There was a problem hiding this comment.
Que es mdash? No lo veo definido en ningun sitio.
There was a problem hiding this comment.
esto... copy paste de /helpers/application_helper.rb función seconds_to_hm(seconds) 😬 me falto tampien copy paste de
def mdash
raw "—"
end
There was a problem hiding this comment.
Vas a necesitar raw tambien me temo.... y creo que es cosa de vistas...
There was a problem hiding this comment.
Ademas, no dupliques codigo. Muevelo y usa en todos sitios la clase esa.
There was a problem hiding this comment.
Me temo que estás haciendo de Ruby aquí. Si no me equivoco #strftime hace esto ya http://ruby-doc.org/stdlib-2.3.1/libdoc/date/rdoc/DateTime.html#method-i-strftime
lib/time_formatter.rb
Outdated
| @@ -0,0 +1,13 @@ | |||
| class TimeFormatter < ActiveRecord::Base | |||
There was a problem hiding this comment.
Este no es un model, no hace falta que herede de ActiveRecord::Base. Es una clase aislada, no tiene ninguna dependencia... creo... o te sirve para algo?
| hours, minutes = minutes.divmod(60) | ||
| raw format("%s%d:%02d", ("-" if sign < 0), hours, minutes) | ||
| else | ||
| mdash |
There was a problem hiding this comment.
Ademas, no dupliques codigo. Muevelo y usa en todos sitios la clase esa.
| #byebug | ||
| # mail notificación pago | ||
| PaymentNotifier.transfer_source(@account.accountable.display_name_with_uid,transfer.amount.to_f/3600).deliver_now | ||
| PaymentNotifier.transfer_destination(@source.accountable.display_name_with_uid,transfer.amount.to_f/3600).deliver_now |
There was a problem hiding this comment.
No, please after_commit no 🙏
| hours, minutes = minutes.divmod(60) | ||
| raw format("%s%d:%02d", ("-" if sign < 0), hours, minutes) | ||
| else | ||
| mdash |
There was a problem hiding this comment.
Me temo que estás haciendo de Ruby aquí. Si no me equivoco #strftime hace esto ya http://ruby-doc.org/stdlib-2.3.1/libdoc/date/rdoc/DateTime.html#method-i-strftime
| #byebug | ||
| # mail notificación pago | ||
| PaymentNotifier.transfer_source(@account.accountable.display_name_with_uid,transfer.amount.to_f/3600).deliver_now | ||
| PaymentNotifier.transfer_destination(@source.accountable.display_name_with_uid,transfer.amount.to_f/3600).deliver_now |
There was a problem hiding this comment.
Eso acolparia mucho el modelo con la notificación y lo pagaríamos caro
|
Seguimos interesados en esto? Si ese es el caso me puedo encargar de acabarlo una vez mergeemos #264 para no abrir demasiados frentes. De todos modos, intentaria centrarnos en limpiar y refactorizar o que haga falta antes de meter nuevas features. |
|
Cerramos esta PR, de momento no necesitamos esta funcioanlidad |
Queremos enviar un mail de notificación cuando se hace un pago de horas, tanto al receptor como al emisor:
Tarea Mail de notificación de pago explicada en http://community.coopdevs.org/t/timeoverflow-siguientes-pasos/139