-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
# Conflicts: # locale/es/LC_MESSAGES/django.po
What's the Google integration look like? I figured Toucan would use OAuth to be able to create a spreadsheet in the user's Google account. |
Yes, Toucan needs OAuth and some other libraries. I add those libraries in requirements.in and I still have the same problem, what can I do? @jpmckinney |
@aguilerapy You need to run: pip-compile
pip-compile requirements_dev.in To update the https://ocds-standard-development-handbook.readthedocs.io/en/latest/coding/index.html#id1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a few things to improve, but in general looks good!
the code needs to be reviewed again
@aguilerapy can you please have a look at the code, and test it locally? I'll send you the test credentials |
def upload_to_drive(request): | ||
|
||
datafile = DataFile(**request.session['google_drive_file']) | ||
credentials = get_credentials_from_session(request) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once a credential is entered into toucan, it cannot be changed, we could create an issue to add an option in which the user can change accounts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can leave that for another issue.
|
||
try: | ||
if credentials: | ||
if credentials.expired: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the credentials expire, it automatically restarts, then the user never enters the credentials again, it's okay?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the credentials are refreshed. The user does not need to give explicit approval again (as long as their credentials are stored in the session).
default/google_drive.py
Outdated
}) | ||
|
||
except Exception as e: | ||
print(e) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This print is correct here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this should return some kind of error message, I'll fix it.
default/static/css/custom.css
Outdated
.content-wrap.ocp-content { | ||
padding-bottom: 2.5rem; | ||
} | ||
.link-disabled { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this class is no longer used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok! I'll remove it.
<ul class="list-unstyled"> | ||
<li> | ||
<strong>{% trans "Upload a file:" %}</strong> | ||
{% trans "Upload no more than one file by dragging and dropping in the box below or using the <strong>Add a file</strong> link." %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This message is shown in all tools, in some functions we can upload more than one file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll check.
</li> | ||
<li> | ||
<strong>{% trans "Provide a URL:" %}</strong> | ||
{% trans "Provide a single URL to a file." %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, in some functions we can provide more than one URL.
prefix = 'flatten-csv' | ||
ext = '.zip' | ||
filename = 'result-csv.zip' | ||
filename = 'result.csv.zip' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
locale/es/LC_MESSAGES/django.po
Outdated
"There was an authorization issue when saving the file to Google Drive, " | ||
"please try again." | ||
msgstr "Se encontró un problema de autorización al guardar el archivo en Google Drive, " | ||
"favor intente de nuevo más tarde" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debemos sacar "mas tarde" o agregar "later" en la traducción?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Creo que hay que decir "please try again later"
#: default/templates/default/mapping_sheet.html:53 | ||
#: default/templates/default/to-spreadsheet.html:25 | ||
#: default/templates/default/to-spreadsheet.html:38 | ||
msgid "File saved on Google Drive" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better to say "in"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
La verdad no estaba segura, así que le pregunté a Teté y me dijo que es "on".
I reviewed the code and tried the new features and I have no further comments. Do you want to look at it? @jpmckinney It's convenient to install the sslserver package. |
I'm happy to trust that it works, given the attention paid to this feature! For deployment, please provide specific, step-by-step instructions for anything needed from Google. |
Hi @jpmckinney, I've followed the instructions here to create the credentials ("Enable APIs for your project" and "Create authorization credentials" sections).
After downloading the client_secret.json file, save it in the root directory for the project with the name "googleapi_credentials.json", or set the Let me know if you need anything else. I think we want to follow the verification process since unverified apps have a limit of 100 new users. The only scope we use is |
It works! 🎉 🎊 |
I was able to use the same OAuth secret I had created earlier around May 20, for which I had requested verification. On https://console.cloud.google.com/apis/credentials/consent?project=toucan-277800 it says:
|
In #42 we consider adding two input/output options, in this pull request we have the output to google drive option, I will create a different one for the URL input option.
For this to work it's necessary to create credentials for the Google Drive API with an OCP account. Here we have a guide.
Then just configure the environment variable:
OCDS_TOUCAN_CREDENTIALS_DRIVE
insettings.py
to point to a JSON file that contains the credentials.We can get this JSON file from here:
![Screenshot_Google Cloud Platform](https://user-images.githubusercontent.com/48607824/72171079-e952cd80-33b0-11ea-91c9-19cb9bbd6518.png)