Storages new folder API endpoint #17375
Conversation
docs/api/apiv3/components/schemas/storage_folder_write_model.yml
Outdated
Show resolved
Hide resolved
docs/api/apiv3/components/schemas/storage_folder_write_model.yml
Outdated
Show resolved
Hide resolved
docs/api/apiv3/components/schemas/storage_folder_write_model.yml
Outdated
Show resolved
Hide resolved
brunopagno
force-pushed
the
impl/59900-storages-new-folder-endpoint
branch
2 times, most recently
from
5d95c73 to
f3782e7
Compare
| .call(storage: @storage, user: current_user, file_id: parent_id) | ||
| .match( | ||
| on_success: lambda { |folder_info| | ||
| path = URI.decode_uri_component(folder_info.location) |
There was a problem hiding this comment.
Not sure if I am missing something here, but I needed to decode URI characters for the fetched information from the provider
There was a problem hiding this comment.
Also on OneDrive, FileInfo#location isn't what you would send up to CreateFolder, it should be id
Annoying? Yes. Terribly so.
There was a problem hiding this comment.
WDYT of moving this method to the create folder service under another name like CreateFolderService.with_parent_id(folder_name, parent_id), so that the resource ends up with no special business logic?
There was a problem hiding this comment.
Moved the logic to the service!
modules/storages/app/services/storages/create_folder_service.rb
Outdated
Show resolved
Hide resolved
| .call(storage: @storage, user: current_user, file_id: parent_id) | ||
| .match( | ||
| on_success: lambda { |folder_info| | ||
| path = URI.decode_uri_component(folder_info.location) |
There was a problem hiding this comment.
Also on OneDrive, FileInfo#location isn't what you would send up to CreateFolder, it should be id
Annoying? Yes. Terribly so.
| .call(storage: @storage, user: current_user, file_id: parent_id) | ||
| .match( | ||
| on_success: lambda { |folder_info| | ||
| path = URI.decode_uri_component(folder_info.location) |
There was a problem hiding this comment.
WDYT of moving this method to the create folder service under another name like CreateFolderService.with_parent_id(folder_name, parent_id), so that the resource ends up with no special business logic?
| Storages::Peripherals::Registry.stub( | ||
| "nextcloud.queries.file_info", | ||
| ->(_) { ServiceResult.success(result: file_info) } | ||
| ) |
There was a problem hiding this comment.
An alternative to just replacing it with a lambda, now that we have validating mocks in place, would be to stub the command/query itself.
It is a bit more verbose, but can help with detecting bad argument passing.
file_info = class_double(Storages::Peripherals::StorageInteraction::Nextcloud::FileInfoQuery)
allow(file_info).to receive(:call).with(valid: :arguments).and_return(ServiceResult.success)
Storages::Peripherals::Registry.stub('nextcloud.queries.file_info', file_info)There was a problem hiding this comment.
Oh, I like this 👏
Will give it a try
There was a problem hiding this comment.
I adjusted the specs here. It actually instantly caught a small issue with my setup. Looking better I think :)
brunopagno
force-pushed
the
impl/59900-storages-new-folder-endpoint
branch
from
1615a1d to
22ea280
Compare
brunopagno
force-pushed
the
impl/59900-storages-new-folder-endpoint
branch
from
22ea280 to
e711b17
Compare
brunopagno
changed the title
Ticket
https://community.openproject.org/wp/59900
What are you trying to accomplish?
The goal is to expose an API for our frontend to call to create new folders on a storage.
What approach did you choose and why?
It's an API endpoint, and a service that call commands that already exist
Merge checklist