zerodha / frappe-attachments-s3

A frappe app to upload file attachments in doctypes to s3.
Other
115 stars 131 forks source link

Got error when attachment is private on the dialog #46

Open gn306029 opened 1 year ago

gn306029 commented 1 year ago

Expected behaviour

Hi. I using this project in my app and my app deploy on AWS EC2 When i in dialog append attachment, if attachment is public just fine But if attachment is private I always got below error

截圖 2022-08-19 上午11 31 45

In email dialog also got below error if attachment is prvate

截圖 2022-08-19 上午11 34 39

This like issue #6

Step to reproduce

  1. Use Dialog API create a new dialog, It must have a attach file field
  2. Select file for attach field. Then ensure is_private is true
  3. Click Upload.

Full Traceback

Traceback (most recent call last):
  File "apps/frappe/frappe/app.py", line 69, in application
    response = frappe.api.handle()
  File "apps/frappe/frappe/api.py", line 55, in handle
    return frappe.handler.handle()
  File "apps/frappe/frappe/handler.py", line 38, in handle
    data = execute_cmd(cmd)
  File "apps/frappe/frappe/handler.py", line 76, in execute_cmd
    return frappe.call(method, **frappe.form_dict)
  File "apps/frappe/frappe/__init__.py", line 1448, in call
    return fn(*args, **newargs)
  File "apps/fab/fab/utils.py", line 123, in make_email
    return _make(
  File "apps/frappe/frappe/core/doctype/communication/email.py", line 169, in _make
    add_attachments(comm.name, attachments)
  File "apps/frappe/frappe/core/doctype/communication/email.py", line 546, in add_attachments
    _file.save(ignore_permissions=True)
  File "apps/frappe/frappe/model/document.py", line 310, in save
    return self._save(*args, **kwargs)
  File "apps/frappe/frappe/model/document.py", line 332, in _save
    return self.insert()
  File "apps/frappe/frappe/model/document.py", line 261, in insert
    self.run_before_save_methods()
  File "apps/frappe/frappe/model/document.py", line 1052, in run_before_save_methods
    self.run_method("validate")
  File "apps/frappe/frappe/model/document.py", line 941, in run_method
    out = Document.hook(fn)(self, *args, **kwargs)
  File "apps/frappe/frappe/model/document.py", line 1260, in composer
    return composed(self, method, *args, **kwargs)
  File "apps/frappe/frappe/model/document.py", line 1242, in runner
    add_to_return_value(self, fn(self, *args, **kwargs))
  File "apps/frappe/frappe/model/document.py", line 938, in fn
    return method_object(*args, **kwargs)
  File "apps/frappe/frappe/core/doctype/file/file.py", line 116, in validate
    self.validate_duplicate_entry()
  File "apps/frappe/frappe/core/doctype/file/file.py", line 241, in validate_duplicate_entry
    self.generate_content_hash()
  File "apps/frappe/frappe/core/doctype/file/file.py", line 276, in generate_content_hash
    frappe.throw(_("File {0} does not exist").format(self.file_url))
  File "apps/frappe/frappe/__init__.py", line 504, in throw
    msgprint(
  File "apps/frappe/frappe/__init__.py", line 479, in msgprint
    _raise_exception()
  File "apps/frappe/frappe/__init__.py", line 434, in _raise_exception
    raise raise_exception(msg)
frappe.exceptions.ValidationError: File /api/method/frappe_s3_attachment.controller.generate_file?key=2022/08/19/Fab PO/0GLVSWOK_20220711.txt&file_name=20220711.txt does not exist

My Tried

Because file.py the validate_url() will check file url is start with /public or /private if not, will check is start will http or https else throw error. Then generate_content_hash() do nothing if file url is start will http . https://github.com/frappe/frappe/blob/version-13/frappe/core/doctype/file/file.py#L290

So I tried to update below code. ensure private url is full url path.

# https://github.com/zerodha/frappe-attachments-s3/blob/master/frappe_s3_attachment/controller.py#L224
file_url = """/api/method/{0}?key={1}&file_name={2}""".format(method, key, doc.file_name)
# To
file_url = """{0}/api/method/{1}?key={2}&file_name={3}""".format(frappe.utils.get_url(),method, key, doc.file_name)

The frappe.utils.get_url() will fetch hostname from site_config.json. if not it will fetch your site name. After update. I avoid occur error in validate_url() or generate_content_hash(). now I can upload private file in s3. In my case, I set the domain for site_config.json the hostname If in the local, hostname maybe can set http://localhost or http://127.0.0.1

Version

Frappe v13.32 ERPNext v13.33

MrGiveItAway-TPK commented 1 year ago

did you find any way to solve this issue u have the same case but when amending a cancelled document @gn306029