-
Notifications
You must be signed in to change notification settings - Fork 29
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
feat(nextcloud): add interceptors #2165
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but can you also migrate the nextcloud_test part that records the fixtures to these new interceptors?
Also we just use our own custom solution instead of https://pub.dev/packages/http_interceptor?
Marking as draft as this interceptor implementation does not allow any caching interceptor. |
I'd like to merge this as is (after fixing the tests and adding separate ones for the interceptors). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but maybe you could also migrate the WebDavCsrfClient to be an interceptor? Can be done as a follow-up though.
Signed-off-by: Nikolas Rimikis <[email protected]>
Signed-off-by: Nikolas Rimikis <[email protected]>
Signed-off-by: Nikolas Rimikis <[email protected]>
Signed-off-by: Nikolas Rimikis <[email protected]>
Signed-off-by: Nikolas Rimikis <[email protected]>
Users should migrate to the CookieInterceptor Signed-off-by: Nikolas Rimikis <[email protected]>
1974b00
to
d995854
Compare
The CI failure looks like the cookies are not applied |
@Leptopoda close this in favor of #2288? |
I separated commits per package to make the changelogs cleaner.
I can do separate PRs if it helps for reviewability (although I think it's quite clean).