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

Add attached files to report #26 #74

Closed
wants to merge 16 commits into from
Closed

Add attached files to report #26 #74

wants to merge 16 commits into from

Conversation

mapeveri
Copy link

#26

@@ -188,6 +189,7 @@ class Mention(SoftDeletableModel):
relevant_audience = models.BooleanField(default=False)
update_rate = models.IntegerField(max_length=1, choices=UPDATE_RATE)
remarks = models.TextField(null=True, blank=True)
upload_file = models.FileField(upload_to="files/", default="", validators=[valid_extension], null=True, blank=True)
Copy link
Member

Choose a reason for hiding this comment

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

@mapeveri por favor asegúrate de que el nuevo código sea compliant con PEP8. Esta línea se ve bastante larga.

Copy link
Member

Choose a reason for hiding this comment

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

@mapeveri de esta manera estamos permitiendo solo un archivo como adjunto a un Mention. Por favor vamos a realizar los cambios necesarios para que se puedan adjuntar varios archivos a un mismo Mention.

Copy link
Author

Choose a reason for hiding this comment

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

Ok lo del PEP8.

Con respecto a lo de los adjuntos es lo que habíamos quedado. Yo te mande un email justamente explicando eso y me dijiste que sonaba bien. Este es una parte del email:

"Te hago algunas consultas:

  1. Solo es un archivo que puede subir no?.
  2. Si hay un archivo y agrega otro, eliminó el anterior?.
  3. El archivo como lo muestro?. Simplemente pongo un icono para que lo descargue o muestro el contenido, si es una imagen es fácil, si es un pdf puedo poner un icono para descargar. No se...
    "

Y vos me pusistes: "Tal como lo describes suena bien."

Copy link
Author

Choose a reason for hiding this comment

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

"por favor asegúrate de que el nuevo código sea compliant con PEP8. Esta línea se ve bastante larga."

De esto ya hice el push, queda pendiente la respuesta de los varios adjuntos.

Copy link
Author

Choose a reason for hiding this comment

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

@sebasmagri con respecto a los adjuntos hable con Berni y quedamos que por ahora soporte un solo archivo por mención. Dependiendo del uso que le de la gente, vemos si se agrega soporte a múltiples archivos o no.

@sebasmagri
Copy link
Member

Hola @mapeveri.

Muchísimas gracias por tu trabajo.

Te dejé algunos comentarios sobre los cambios. Por favor déjame saber si tienes alguna duda al respecto.

Cuando tengas los cambios, por favor hazle squash con los commits que ya subiste y actualiza el PR.

Saludos!

@mapeveri
Copy link
Author

@sebasmagri ahí deje varios comentarios.

Gracias

@mapeveri
Copy link
Author

@sebasmagri ya hice algunos cambios de lo que me pediste, queda que me respondas algunas preguntas que te hice para seguir.

Muchas gracias

@mapeveri
Copy link
Author

@sebasmagri decime si esta ok todo, o falta algo más para dar por finalizado este issue.

Gracias!

@sebasmagri
Copy link
Member

Bien @mapeveri. Puedes hacer squash de todos los commits del Pull Request en uno solo?

Saludos.

@mapeveri
Copy link
Author

@sebasmagri creo que ahí estaría. Sinó es así, me dirías cual es el comando exactamente. Gracias

@sebasmagri
Copy link
Member

@mapeveri dale una revisada a git rebase -i.

Add attached files to report - pull request

Add attached files to report #26

Add attached files to report #26

Add attached files to report #26

Add attached files to report #26

Issue 26
@mapeveri
Copy link
Author

@sebasmagri genial ahí me dio el mensaje "Successfully rebased and updated refs/heads/development." Así que creo que estaría ok.

# The first commit's message is:
Issue 26

# This is the 2nd commit message:

Issue 26

# This is the 3rd commit message:

Add attached files to report #26

# This is the 4th commit message:

Add attached files to report #26

# This is the 5th commit message:

Add attached files to report #26

# This is the 6th commit message:

Add attached files to report #26

# This is the 7th commit message:

Add attached files to report - pull request
@mapeveri mapeveri closed this Apr 27, 2016
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.

None yet

2 participants