-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Feature: Display conflicts modal when pasting in FTP locations #12242
Feature: Display conflicts modal when pasting in FTP locations #12242
Conversation
Can you make |
I think it would cause issues once injected. We would have two different instances of the interface |
What are the differences between the services? |
There are no differences, but we would inject two IStorageService instances, Native and Ftp. I think this might cause some issues when retrieving them. I'll try and see what happens |
I meant, why did you create IFtpStorageService? |
Because I thought that if I have to retrieve it as IStorageService, but I have two instances of that (NativeStorageService and FtpStorageService), there would be issues since it may give me once Native and once Ftp Also, I thought I should keep the pattern service interface - service implementation |
Maybe instead of duplicating
I'm more into the second option because the first one would introduce more mess. The only difficulty with the second option is the injection itself -- the UI would need to inject appropriate implementation into the View Model Perhaps you have something else on your mind? @ferrariofilippo |
Not really, I'm not really an expert on this, that's why I used this bad "hack" |
I need |
You could inject it by passing it as a parameter however, then we'd have no guarantee that appropriate service was passed. Unless a major refactor in this area is done, that's going to be an issue. We could perhaps continue with your approach, but to bit reduce code duplication and increase reusability you could inherit from public interface IFtpStorageService : IStorageService |
Resolved / Related Issues
Closes Feature: Display conflicts modal when pasting on FTP locations #8609
Related Fix: Fixed a crash with ftp connections #12228 (Files would still crash if the root
/
wasn't specified [e.g.ftp://localhost:21
])Changes
FtpManager
), so I left only oneIFtpStorageService
which is essentially a copy ofIStorageService
. I might remove it and inject the service without its relative interface. @yaira2 what are your thoughts on this?Limitations
Pasting from FTP to FTP isn't showing the conlicts dialog (I'm working on this)FixedValidation
How did you test these changes?