Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Se cambiaron las platillas a slim y se integra formulario de Student Application #3

Closed
wants to merge 6 commits into from

3 participants

@jdsampayo

Se integro authlogic en favor de devise por hijack de refinery en el modelo User
Se agrega modelo Customer para login
Se agrega modelo CustomerSession para la sesion
Se integro cancan para los permisos

Correcciones a las paginas originales en slim, se agregan placeholders

Se agrego modelo Student
Formulario de Student Application

@jdsampayo jdsampayo Se cambiaron las platillas a slim
Se integro authlogic en favor de devise por hijack de refinery en el modelo User
Se agrega modelo Customer para login
Se agrega modelo CustomerSession para la sesion
Se integro cancan para los permisos

Correcciones a las paginas originales en slim, se agregan placeholders

Se agrego modelo Student
Formulario de Student Application
7b48bc5
@jdsampayo

Hola,

Solo un detalle, en lugar de usar Devise mejor meti Authlogic porque se me presentó este problema:
http://stackoverflow.com/questions/16353497/problems-adding-devise-model-to-a-refinerycms-app

Refinery sobreescribe el modelo User, entonces como de todas formas ibamos a manejar el login a parte para los Students, Project Manager y quienes crean las Paginas, decidi integrar Authlogic con modelos con otro nombre Customer y CustomerSession.

Hay una solución para lo de Refinery y usar tu propio Devise en teoría, pero requiere muchos hacks y a la larga si llegan a actualizar la version de Refinery seguramente va a tronar algo, te la dejo por si quieres hecharle un ojo para ver si vale la pena cambiarlo despues:
http://sdownie.com/articles/refinerycms-rails-3-2-into-your-existing-app

Pero al ver como queda el modelo User y los module_eval que hay que hacer, se me hizo muy complicada.

@laspluviosillas

Gracias Jorge! Al rato lo checo a fondo pero de una revisada general el unico pero que le pongo es la integración de SASS. Podemos mantener todo con SCSS?

@jdsampayo

Sin problema, voy a cambiar ese archivo

@jdsampayo

Ya quedo!

@laspluviosillas

Ok ya pude revisar esto mas a fondo. En general esta muy bien :+1:, se ve que fue bastante trabajo. La mayoria de mis peros son de cosas que realmente no explicamos a fondo, entonces las confusiones son entendibles. Gracias Jorge! Pero bueno, aqui van los detalles:

Uso de Customer como User

Entiendo que tuviste que escoger otro nombre de modelo porque User es usado por Refinery, y Customer es una buena adivinanza, pero como todo esto es en un entorno no-lucrativo siento que "cliente" realmente no aplica. Yo preferiria algo mas generico como Login, @lapluviosilla alguna sugerencia?

Uso de ProjectSession.

Hay dos tipos de usuarios: los que crean proyectos y los que aplican a proyectos. El proyecto en si no es un usuario, sino el creador de proyectos lo es. Todo lo de ProjectSessions deberia ser algo como ProjectManagerSessions. @lapluviosilla alguna sugerencia?

Modelo StudentApplication.

Ahorita todos los detalles del student application los estas agregando al modelo Student. Por ahorita esta bien, pero creo que tenemos que separar la aplicacion a un modelo StudentApplication. Un Student solamente puede tener un Project activo pero es posible que tenga proyectos historicos, o sea proyectos que ha realizado en un pasado. Si guardamos los valores del "student application" en el modelo Student, no nos permitiria mantener mas que un Project guardado por Student.

Yo propongo que el StudentApplication sea la tabla de join entre Student y Project para que sea una relacion many-to-many. Podriamos usar el patron find_or_create para asociar nuevos StudentApplication's a Students o que los crea al vuelo.

Refactorizacion de los define_methods en Student

Student tiene unos usos de define_method "#{f}_with_deserialize" que son identicos a los que se usan en Project. Es un detalle menor pero es duplicacion de codigo y podriamos refactorizar esto para que ocupe un metodo estandarizado para los dos modelos.

app/controllers/application_controller.rb
@@ -7,4 +8,26 @@ class ApplicationController < ActionController::Base
def set_locale
I18n.locale = http_accept_language.compatible_language_from(I18n.available_locales)
end
+
+ def current_customer_session
+ return @current_customer_session if defined?(@current_customer_session)
+ @current_customer_session = CustomerSession.find
+ end
+
+ def current_customer
+ return @current_customer if defined?(@current_customer)
+ @current_customer = current_customer_session && current_customer_session.record
+ end
+
+ # CanCan needs it
+ alias_method :current_user, :current_customer
@laspluviosillas Owner

Esto no causa conflicto con refinery? He usado CanCan en el pasado con modelos que no son "User" entonces deberia de haber una forma de que CanCan use current_customer directamente.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
app/controllers/customer_sessions_controller.rb
@@ -0,0 +1,37 @@
+# coding: UTF-8
+
+class CustomerSessionsController < ApplicationController
+ def new
+ @customer_session = CustomerSession.new
+ end
+
+ def create
@laspluviosillas Owner

Aqui dependiendo del tipo de customer que se esta logueando, guardas el id en sesion como student_id o project_id. No permitiria esto tecnicamente que el usuario tuviera dos sesiones abiertas? Uno como student y otro como project? Probablemente no llegue a pasar pero podria ocasionar un bug.

No, una Sesion tiene un solo Usuario, y un Usuario tiene una sola Entidad, la entidad puede ser un Student o un ProjectManager, lo cambio por LoginSession para evitar la confusion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@laspluviosillas

Por ultimo, otro detalle muy menor es que muchos de los bloques respond_to podrian ser re-emplazados con bloques respond_with

@laspluviosillas laspluviosillas commented on the diff
app/controllers/students_setup_controller.rb
((30 lines not shown))
+ @student.build_spiritual_reference unless @student.spiritual_reference
+ @student.build_academic_reference unless @student.academic_reference
+ end
+
+ params[:student][:wizard_status] = step.to_s
+ params[:student][:wizard_status] = 'complete' if step == steps.last
+
+ #This is saving two times, one here and another in render_wizard
+ @student.update_attributes params[:student]
+
+ render_wizard @student
+ session[:student_id] = @student.id
+ end
+
+ private
+ def current_student
@laspluviosillas Owner

No seria esto un metodo para ApplicationController para que lo podamos usar globalmente?

Ahh este codigo lo clone del projects_setup_controller, es solo para el Wizard, en el Application tendria que ser un poco diferente, porque dependeria de Customer (Login proximamente) no de la session que solo usa el Wizard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@lapluviosilla
Collaborator

Jorge gracias por tu trabajo! Por la mayor parte se ve bien. Solo voy a agregar unos comentarios a base de lo que comento Jaime.

Uso de Customer como User y el Authentication Provider

La guia a la que apuntaste es para integrary el usuario de refinery dentro de un usuario existente. Pero nosotros queremos mantener los 2 usuarios completamente aparte. Entiendo que hay un conflicto con respeto a los devise routes, pero Jaime y yo creemos que si le damos un poco de tiempo se podria encontrar solucion para este problema. En si quisieramos tener un RefineryUser y User.

La razones que preferimos solo usar Devise y no meter a Authlogic son estas:

  1. Simplemente es mucho bloat/overhead tener 2 authentication frameworks en un solo sistema, y si es posible queremos evitar esto.
  2. En el futuro hay la posibilidad de que vamos a querer agregar salesforce authentication al app. Ya existe un plugin para devise que nos permite autenticar usuarios de salesforce por medio de devise y OAuth. Que yo sepa no existe algo similar para Authlogic, y por lo general hay mas plugins disponibles para authlogic.

Este punto de integracion va ser un poco dolorosa pero creo que vale la pena hacerlo mas optimo.

Uso de ProjectSession.

Si, como todavia no definimos los roles entiendo que podria haber confusion aqui. En si como dice Jaime no habria un ProjectSession. A lo mejor algun "current project application" que se guarde en el user session, asi como se hace con el project, pero no estoy seguro.

Modelo StudentApplication.

Estoy de acuerdo con lo que dice Jaime. Con el proyecto era un poco diferente porque por el momento el proyecto es tambien un application. A lo mejor tengamos que refactorizar eso ya que se complique el proceso de aplicacion, pero por ahora sirve.

Pero con el estudiante si necesita a fuerzas su propio applicacion. En si el estudiante esta aplicando a un proyecto, no es una aplicacion general de estudiante.

A lo mejor lo debemos llamar StudentProjectApplication o algo por el estilo.

Specs

No hemos hecho un buen trabajo de hacer specs por lo pronto, pero hay que empezar a ver esto antes de que la aplicacion se vuelva muy grande y sea demasiado grande la tarea de regresar y hacer specs/tests.

Cuando pueda voy a reevaluar lo que tenemos disponible para tests, pero por lo pronto podrias agregar unos specs muy basicos para los modelos?

@jdsampayo

Uso de Customer como User
Por ahora lo cambio a Login con Authlogic y voy revisando la integracion con devise (debido a que ambos usan current_user) no veo que impacte mucho en las demás secciones pero si se considera el cambio a Devise.

Uso de ProjectSession
Creo que lo estan confundiendo con CustomerSession, mejor lo cambio por ahora por LoginSession, el Usuario si considera que pueda ser de varios tipos, asi que sera un solo LoginSession.
ProjectSession se refiere a las fechas de un Project, es decir el start y el end, porque en las pantallas, un Project puede tener como que varias sesiones (fechas de proyecto), me imagino proyecto para irse en Primavera y en Verano, algo por el estilo, pero no se refiere a la session de Login, sino a la session de tiempo que estara ahciendose el proyecto.

Modelo StudentApplication
Esta parte si tendria que pensar un poquito más el impacto pero sí, que un Student pueda tener varios Projects.

Refactorizacion de los define_methods en Student
Esta parte yo creo que la dejamos como una tarea a parte porque se va a tener que mover en ambos para que no jale el nombre del input del serialize y se pueda usar para ambos, por que si, se estan duplicando tambien en los locales yml las opciones.

Specs
Agrego los basicos pero la verdad no tengo tanta habilidad en los tests, necesitaria algunos ejemplos o que lo revisaran una vez que suba los basicos.

@lapluviosilla
Collaborator

Jorge, por ahora vamos a crear un demo branch para demonstrar lo que llevamos al cliente, asi que por mientras sigue trabajando en este branch y PR. Todavia hay detalles que queremos mejorar aunque ya una gran parte de la funcionalidad esta ahi.

Sigue el buen trabajo! Yo creo que le va dar gusto al cliente cuando le mostremos hoy lo que se ha hecho.

Nested Route and Refactoring

Asi como mencionamos en el correcto nos pareceria bien si la aplicacion existiera como /projects/water_africa/apply, etc. Por favor restructura los controllers y routes de esta forma. Tambien seria bueno si podrias agregar un "Apply" button al projects list page (/projects) y asi ir directamente a la student aplicacion para ese proyecto
en vez de tener que seleccionarlo en un drop down box. Se que eso no aparece en el wireframe, pero creo que es a lo que queremos ir. De hecho creo que se necesita quitar el project select drop down box del wireframe y solo desplegar el nombre del proyecto al que estas aplicando asi como se hace en las demas paginas.

Tambien a la mano con esto va la modificacion de los modelos para que haya un StudentProjectApplication aparte del Student.

Como ves esto no se definio bien, asi que no hay problema con la forma que lo has hecho, pero mientras lo vamos viendo y platicamos con el cliente estos son los cambios que se requieren.

Validacion

Parece que hay varias problemas de validacion. No todos los campos se estan validando. Todos los campos que aparecen con una estrella gris en el wireframe son required. Usa el project application como ejemplo de como validamos los fields en diferentes pasos.

Por favor asegura que todos los campos se estan validando. Tambien me permite ir al siguiente paso sin poner un project session lo cual causa un error en la proxima pagina. Podrias agregar unos project sessions al seeds.rb para que esten prepopulados? En algun momento sincronizaremos esos con Salesforce, pero por lo pronto los podemos poner en seeds.

Tambien hay un problema que descubri que ocurre cuando pones un email pero no un password. Aqui esta la foto:
failedvalidation

@jdsampayo

Claro que si Paul,

Revizo estos cambios y este bug

@jdsampayo

Se ha cambiado Customer por Login, así como CustomerSession por LoginSession.

@jdsampayo

Se corrigió problema de validación del password

@laspluviosillas laspluviosillas deleted the dev branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 2, 2013
  1. @jdsampayo

    Se cambiaron las platillas a slim

    jdsampayo authored
    Se integro authlogic en favor de devise por hijack de refinery en el modelo User
    Se agrega modelo Customer para login
    Se agrega modelo CustomerSession para la sesion
    Se integro cancan para los permisos
    
    Correcciones a las paginas originales en slim, se agregan placeholders
    
    Se agrego modelo Student
    Formulario de Student Application
  2. @jdsampayo
Commits on Dec 3, 2013
  1. @jdsampayo

    removed username

    jdsampayo authored
Commits on Dec 17, 2013
  1. @jdsampayo
  2. @jdsampayo
  3. @jdsampayo

    corrected validations

    jdsampayo authored
Something went wrong with that request. Please try again.