refactor: File #16289
Merged
refactor: File #16289
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## develop #16289 +/- ##
===========================================
- Coverage 58.49% 53.11% -5.38%
===========================================
Files 772 758 -14
Lines 69190 67837 -1353
Branches 6012 5749 -263
===========================================
- Hits 40471 36030 -4441
- Misses 25091 27736 +2645
- Partials 3628 4071 +443
Flags with carried forward coverage won't be shown. Click here to find out more. |
* Separate out utils, exceptions & core / maintain import paths [except API endpoints] * Add type hints * Simplify logic
Other fix: * Don't set File.file_name (too) different from File.file_url
Restructured and moved most APIs under frappe.core.api.file namespace.
Changed some obvious security gaps (like using get_list instead of
get_all for an endpoint), styled, added type hints and made minor performance
enhancements.
Changes
* download_file API
* Move API to handler.py
* Check for permissions via File.is_downloadable instead
* Moved APIs to new namespace: `frappe.core.api.file`
* Backwards compatibility
* Added APIs to override_whitelisted_methods to maintain existing
client endpoints
* Imported APIs to controller's namespace to avoid breaking external
app usages
The old paths worked too, but it's just better to use the new paths and not go in circles ;)
* Simplify object lifecycle flows * Re-organize and separate distinct actions / APIs * Style black-ish * Simplify logic in methods "touched"
gavindsouza
force-pushed
the
refactor-file
branch
from
0325897
to
3110c42
Compare
|
@gavindsouza since you are cleaning up file, please also fix the bug where if you select a file as a property in a document, it creates a new file record. (example: attach a file to a blog, and then select it as cover - 2 files records get created)! Had discussed this with @surajshetty3416 |
* Generalize file content reverting on rollback * Overwrite current file instead of adding a dangling new file * Use File.save_file instead of additional "custom" logic * Set File.content on calling get_content method
* Make file renaming atomic * Add rollback observer to maintain filesystem consistency * Simplify logic
* Set default Falsy value for content * Extend Frappe exceptions into file's
Delete, Move or Restore files in the filesystem by setting Document flags in the on_rollback method
* Move checks into common methods * Refactor to use latest APIs * Remove seemingly unnecessary code * Fix previously undhanled use cases
To check out the up-to-date version of hooks.md, see https://frappeframework.com/docs/v13/user/en/python-api/hooks
gavindsouza
force-pushed
the
refactor-file
branch
from
9c7a543
to
47cf46c
Compare
|
Fixed #13624 by ^ |
|
Shall muster the courage to revisit soon 🥲 |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing. |
gavindsouza
force-pushed
the
refactor-file
branch
from
2ad7596
to
5ec2690
Compare
it knows * Hide Preview if request is errored. EAFP. * Set file attachment limit based on how many attachments have already been added. If 1 attachment exists and the limit is 3, File uploader will allow you to select only 2 more files.
gavindsouza
force-pushed
the
refactor-file
branch
from
1fc9d97
to
e22ab86
Compare
|
Ignoring flaky Cypress test |
This was referenced Jul 18, 2022
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes
./frappe/core/doctype/file/file.py-> [./frappe/core/api/file.py,./frappe/core/doctype/file/utils.py,./frappe/core/doctype/file/exceptions.py,./frappe/core/doctype/file/file.py]/api/method/frappe.core.api.fileFixes
File.file_nameattributeOther Changes
test_multiple_doctypes_syncfor Postgres test run