-
Notifications
You must be signed in to change notification settings - Fork 81
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
Support cyrus server #3457
base: master
Are you sure you want to change the base?
Support cyrus server #3457
Changes from 12 commits
7a2380c
5d91050
8d763f8
04c97c0
58160a6
99b53ac
6d24e0c
f948ed5
da3cd7b
fa3008d
1958e96
c960db5
cc1e503
b4ad37f
fc04807
9d14de7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ void main() { | |
final baseUrl = Uri.parse('https://domain.com'); | ||
final sourceUrl = Uri.parse('/jmap/'); | ||
|
||
final qualifiedUrlExpected = Uri.parse('https://domain.com/jmap'); | ||
final qualifiedUrlExpected = Uri.parse('https://domain.com/jmap/'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why we need this? In what cases? IMO, it is our normalization, please keep it in uniform. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cyrus advertizes the following in the session: when querying There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okie, in this case, make it uniform, keep There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But tmail-backend works the other way around: no So I feel like the solution is to keep the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
yes. Update tests to include it |
||
final qualifiedUrlResult = sourceUrl.toQualifiedUrl(baseUrl: baseUrl); | ||
|
||
expect(qualifiedUrlResult, equals(qualifiedUrlExpected)); | ||
|
@@ -50,7 +50,7 @@ void main() { | |
final baseUrl = Uri.parse('https://domain.com/jmap'); | ||
final sourceUrl = Uri.parse('/'); | ||
|
||
final qualifiedUrlExpected = Uri.parse('https://domain.com/jmap'); | ||
final qualifiedUrlExpected = Uri.parse('https://domain.com/jmap/'); | ||
final qualifiedUrlResult = sourceUrl.toQualifiedUrl(baseUrl: baseUrl); | ||
|
||
expect(qualifiedUrlResult, equals(qualifiedUrlExpected)); | ||
|
@@ -70,7 +70,7 @@ void main() { | |
final baseUrl = Uri.parse('https://domain.com:2000/jmap'); | ||
final sourceUrl = Uri.parse('https://domain.com:2001/jmap/'); | ||
|
||
final qualifiedUrlExpected = Uri.parse('https://domain.com:2001/jmap'); | ||
final qualifiedUrlExpected = Uri.parse('https://domain.com:2001/jmap/'); | ||
final qualifiedUrlResult = sourceUrl.toQualifiedUrl(baseUrl: baseUrl); | ||
|
||
expect(qualifiedUrlResult, equals(qualifiedUrlExpected)); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ import 'package:core/presentation/state/success.dart'; | |
import 'package:dartz/dartz.dart'; | ||
import 'package:dio/dio.dart'; | ||
import 'package:model/upload/file_info.dart'; | ||
import 'package:model/error_type_handler/unknown_uri_exception.dart'; | ||
import 'package:tmail_ui_user/features/composer/domain/repository/composer_repository.dart'; | ||
import 'package:tmail_ui_user/features/composer/domain/state/upload_attachment_state.dart'; | ||
|
||
|
@@ -13,10 +14,11 @@ class UploadAttachmentInteractor { | |
|
||
Stream<Either<Failure, Success>> execute( | ||
FileInfo fileInfo, | ||
Uri uploadUri, { | ||
Uri? uploadUri, { | ||
CancelToken? cancelToken, | ||
}) async* { | ||
try { | ||
if (uploadUri == null) throw UnknownUriException(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should not put the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is what I did at first, but it means the interactor is not even called in case of a I thought it would be better for the UX to have the red toast notification show up, but I understand the concern. Should I change it to put the check before ? Maybe I can throw a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, you check There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done ! |
||
final uploadAttachment = await _composerRepository.uploadAttachment( | ||
fileInfo, | ||
uploadUri, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -353,9 +353,12 @@ class IdentityCreatorController extends BaseController with DragDropFileMixin im | |
void _getAllIdentitiesSuccess(GetAllIdentitiesSuccess success) { | ||
if (success.identities?.isNotEmpty == true) { | ||
listEmailAddressDefault.value = success.identities! | ||
.where((identity) => (identity.email != null && identity.email!.isNotEmpty)) | ||
dab246 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.map((identity) => identity.toEmailAddressNoName()) | ||
.toSet() | ||
.toList(); | ||
|
||
listEmailAddressDefault.add(EmailAddress(null, session?.getOwnEmailAddress())); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So you should check if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, it is already good, there is already no duplication. |
||
listEmailAddressOfReplyTo.add(noneEmailAddress); | ||
listEmailAddressOfReplyTo.addAll(listEmailAddressDefault); | ||
_setUpAllFieldEmailAddress(); | ||
|
@@ -375,11 +378,9 @@ class IdentityCreatorController extends BaseController with DragDropFileMixin im | |
void _setDefaultEmailAddressList() { | ||
listEmailAddressOfReplyTo.add(noneEmailAddress); | ||
|
||
if (session?.username.value.isNotEmpty == true) { | ||
final userEmailAddress = EmailAddress(null, session?.username.value); | ||
listEmailAddressDefault.add(userEmailAddress); | ||
listEmailAddressOfReplyTo.addAll(listEmailAddressDefault); | ||
} | ||
final userEmailAddress = EmailAddress(null, session?.getOwnEmailAddress()); | ||
listEmailAddressDefault.add(userEmailAddress); | ||
listEmailAddressOfReplyTo.addAll(listEmailAddressDefault); | ||
|
||
_setUpAllFieldEmailAddress(); | ||
} | ||
|
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.
What happens if
parse
failed? Can we usetryParse
instead?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.
any update? @florentos17
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.
What default behaviour would you expect if the parse fails and
null
is returned ?The methods that call
toQualifiedUrl
(getQualifiedApiUrl
,getDownloadUrl
,getUploadUrl
) mostly cannot work withnull
, so errors have to be thrown. But then it would be easier to just lettoQualifiedUrl
throw the error ?