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

Fix bugs I found in this app #3

Open
wants to merge 58 commits into
base: master
Choose a base branch
from
Open

Fix bugs I found in this app #3

wants to merge 58 commits into from

Conversation

chitly
Copy link

@chitly chitly commented Apr 22, 2022

Thank you a lot, for improving this app.
After I use this repo, I found 3 bugs, then I fix them and want to share them with your repo.

  1. After I migrate existing files, I found some files I manually remove, so I add the code to check the file exists on disk.
if not s3_file_regex_match(file['file_url']) and is_file_exists(file['name']):
    upload_existing_files_s3(file['name'], file['file_name'])
    return True
def is_file_exists(file_name):
    if frappe.db.exists('File', {'name': file_name}):
        doc = frappe.get_doc('File', file_name)
        return doc.exists_on_disk()
    return False
  1. When I uploaded the file to AWS S3, then I have an error about file_name in my language, then I fix it.
import urllib
encoded_file_name = urllib.parse.quote(file_name)
params['ResponseContentDisposition'] = 'filename={}'.format(encoded_file_name)
  1. Finally, I uploaded the file in my doc, but the URL didn't change, then I found this PR, and it work to fix this problem.
frappe.db.sql("""UPDATE `tabFile` SET file_url=%s, folder=%s,
    old_parent=%s, content_hash=%s WHERE name=%s""", (
    file_url, 'Home/Attachments', 'Home/Attachments', key, doc.name))
doc.reload()

@Namekkural
Copy link

I would love to see these fixes implemented in to the parent branch. For now I'm using you repo @chitly

@pythonpen
Copy link

pythonpen commented Jun 4, 2022

Thanks for the contributions @chitly- will merge them soon ;)

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.

3 participants