add aliases feature #1523

tahaalibra · 2016-06-08T11:47:09Z

this will solve #122
this will allow sending email from aliases

Note:
~~much of it is incomplete~~

Time to review

@ChristophWurst @Gomez

ChristophWurst · 2016-06-08T12:18:23Z

Looks promising!

ChristophWurst · 2016-06-09T12:52:19Z

(copied from #1500)

Account settings - Aliases #122

DB design/implementation (schema, mapper, entity)
Controller/service adjustments
UI design (mockup, gui)
Client-side implementation
- Alias configuration (account setup/settings view)
- Alias selection when sending new mails
Server-side adjustments for sending mails and storing drafts (I guess we'll need to fix that)
Validation of alias email / check if it already existsl

ChristophWurst · 2016-06-10T07:05:56Z

I'd like to talk about the DB schema design. What was the motivation to model it that way? I'm pretty sure we should re-think that and create a true 1:n relation of account:alias. Hence, the alias table would look like id, account_id and alias.

Gomez · 2016-06-10T07:28:36Z

Correct, email is a duplicate. @tahaalibra You agree?

tahaalibra · 2016-06-10T08:55:11Z

yes thank makes sense,

i will update this in the next PR

ChristophWurst · 2016-06-10T08:58:35Z

@tahaalibra what would we need the user_id in the aliases table for? Isn't that redundant?

i will update this in the next PR

please use this PR so we have the whole alias feature in one single PR.

tahaalibra · 2016-06-10T09:02:10Z

@ChristophWurst yes i mean update in the next commit, my bad ;)....i think you are right user_id does seem redundant.

ChristophWurst · 2016-06-10T09:04:32Z

when adding new commits, please try to use meaningful commit messages. That makes reviewing easier ;-)

ChristophWurst · 2016-06-12T09:07:34Z

appinfo/database.xml

+				<notnull>true</notnull>
+				<length>255</length>
+			</field>
+		</declaration>


👍 schema looks good

tahaalibra · 2016-06-14T20:41:13Z

new commit, this is still very unstable, fixed most of the previous bugs

cc @ChristophWurst @Gomez

tahaalibra · 2016-06-18T22:15:49Z

sending mail with aliases is working (tested using mail setting provide by @Gomez )

to send mail using alias

head over to account setting
then add alias email
after that head over to inbox and refresh the webpage( currently refreshing is used to fetch new alias from the DB, this is temporary),
head over to composer and a send mail :)

@ChristophWurst @Gomez

Gomez · 2016-07-22T14:49:53Z

@tahaalibra Is something blocking you? We are near to merge, would be cool to get in done soon. (Btw. i use this branch nearly every day :)))

ChristophWurst · 2016-07-28T13:21:50Z

js/views/composer.js

@@ -509,6 +509,11 @@ define(function(require) {
 					alias =  _.find(this.aliases, function(alias) {
 						return alias.emailAddress  === require('state').currentAccount.get('email');
 					});
+				} else {
+					var firstAccount = this.accounts.at(1);


if you add one account and then another, the accounts are ususally ordered as account1, unified account, account2. Therefore you'd select the unified inbox, although you actually want the first account. Use something like this.accounts.filter(…accountId not -1…).at(0).

ChristophWurst · 2016-07-29T09:44:50Z

please fix CI errors, otherwise this can not be merged

ChristophWurst · 2016-07-29T09:47:01Z

I can not delete aliases:
ReferenceError: event is not defined, aliases-list.js:40:4

ChristophWurst · 2016-07-29T09:47:51Z

tested with a single account configured and it still does not select an email address by default :-(

jancborchardt · 2016-07-30T09:08:29Z

It would be good to merge this PR as soon as possible and work on the additional stuff in follow up pull requests. Otherwise it gets too big!

add aliases feature add aliases feature mapper entity service and controller for aliases comments and fixes minor bug fix minor changes aliases js service, database update and some security fixes minor fixes for saving and loading aliases send mail from composer using alias small fixes fix most test, delete alias work, better ui set an alias name with alias id using alias name when sending email works big fixes big fixes reply composer using aliases bug fixes and aliases collection minor bug fixes minor fix radio for aliases controller and service fix for when refrshing settings page fix email length in the database [tx-robot] updated from transifex change to secure connection remove secure http: false [tx-robot] updated from transifex Correct return for getFolderById - fixes #1514 account !== folder fetch UIDs and DATEs of all messages and do the pagination client side use search strategy only if SORT is not supported update changelog for 0.5.2 Update composerlock to https remove more message button is obsolete with infinite scroll [tx-robot] updated from transifex [tx-robot] updated from transifex [tx-robot] updated from transifex refactor message 'load' event parameters messagecontent -> foldercontent pass account/folder via constructor also use the new collection when refreshing inject messages collection Fix jscs errors sync karma require config with runtime require config open first mailbox when clicking the account's email address fix jscs error update grunt 0.5.3 version bump update/unify license headers use underscore to debounce search filters; do not re-search the same term remove obsolete folder reset event + event handler smoother transition when adding another account remove user id from alias table and js bug fix and optimization add new keyboard channel and port existing key listener allow navigation with j, k allow left/right arrow to switch messages use 'mail' instead of 'Mail' for bower to prevent warning merge conflict resolve squash and rebase commits fix AccountsControllerTest::testIndex travis js fix alias collection update adding aliases collection and bug fixes big fixes show you when mail is sent to an alias or email select current account as default in composer fix for selecting default account on composer fix grunt issues fixes and improvments remove old code that prevented selecting default account on composer ui improvment

ChristophWurst · 2016-08-03T08:17:49Z

Small enhancement for a follow-up PR:

Hide account dropdown of the composer if there's only one email address (one account, no aliases)

ChristophWurst · 2016-08-03T08:38:25Z

👍 from my side. Nice job @tahaalibra!

@jancborchardt another quick testing/review?

jancborchardt · 2016-08-03T09:08:42Z

When opening an email with multiple people in recipients or cc, only the first person is shown in the »to« field of the reply. In master, all people are shown correctly in to/cc.

👎 til that’s fixed.

jancborchardt · 2016-08-03T09:13:28Z

And in the settings, the placeholder for the input field should be called »Mail address« instead of »Alias«.

If those 2 things are fixed then we can merge. :)

tahaalibra · 2016-08-03T10:36:05Z

@jancborchardt @ChristophWurst can you review the fixes...

ChristophWurst · 2016-08-03T10:55:50Z

Merge branch 'aliases' of https://github.com/owncloud/mail into aliases

What did you merge there?

ChristophWurst · 2016-08-03T10:57:06Z

ah, you merged my commit

jancborchardt · 2016-08-03T12:11:51Z

@tahaalibra the cc issue still exists. Only the person whom the mail is from will be put in the »to«, but all people from cc just vanish.

tahaalibra · 2016-08-03T13:39:54Z

@jancborchardt its working fine on my side, the cc list is showing. Still pushed another commit, try if its working?

jancborchardt · 2016-08-04T12:57:24Z

Works now, but one last issue:

When there are multiple people whom the mail is addressed to, the selected »from« address is always the first one in the dropdown. Not the correct one.

ChristophWurst · 2016-08-04T13:31:35Z

Works now, but one last issue:

When there are multiple people whom the mail is addressed to, the selected »from« address is always the first one in the dropdown. Not the correct one.

Can't reproduce ->merge

jancborchardt · 2016-08-04T14:18:14Z

Ok, guess it might have been only my issue.

tahaalibra added enhancement 2 - developing in progress labels Jun 8, 2016

tahaalibra self-assigned this Jun 8, 2016

ChristophWurst reviewed Jun 12, 2016
View reviewed changes

appinfo/database.xml

<notnull>true</notnull>

<length>255</length>

</field>

</declaration>

Copy link

Contributor

ChristophWurst Jun 12, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 schema looks good

tahaalibra force-pushed the aliases branch from 863cc75 to bdd5a5c Compare July 27, 2016 20:18

ChristophWurst reviewed Jul 28, 2016
View reviewed changes

tahaalibra force-pushed the aliases branch from a535fb2 to 173aeb8 Compare July 29, 2016 17:04

tahaalibra force-pushed the aliases branch from 656db4c to 65e031a Compare August 2, 2016 12:46

ChristophWurst force-pushed the aliases branch from 86edf1f to 2cad308 Compare August 3, 2016 08:37

version bump to trigger db upgrade

2cad308

tahaalibra added 2 commits August 3, 2016 15:51

fix cc bugs and small improvement

e362cf0

Merge branch 'aliases' of https://github.com/owncloud/mail into aliases

0cf2375

fix for showing cc

f4bbbf6

ChristophWurst merged commit 0a3015c into master Aug 4, 2016

ChristophWurst deleted the aliases branch August 4, 2016 13:31

plastikschnitzer mentioned this pull request Sep 23, 2016

0.6-RC1 bugs nextcloud/mail#109

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add aliases feature #1523

add aliases feature #1523

tahaalibra commented Jun 8, 2016 •

edited

Loading

ChristophWurst commented Jun 8, 2016

ChristophWurst commented Jun 9, 2016 •

edited by tahaalibra

Loading

ChristophWurst commented Jun 10, 2016

Gomez commented Jun 10, 2016

tahaalibra commented Jun 10, 2016

ChristophWurst commented Jun 10, 2016

tahaalibra commented Jun 10, 2016

ChristophWurst commented Jun 10, 2016

ChristophWurst Jun 12, 2016

tahaalibra commented Jun 14, 2016 •

edited

Loading

tahaalibra commented Jun 18, 2016 •

edited by Gomez

Loading

Gomez commented Jul 22, 2016

ChristophWurst Jul 28, 2016

ChristophWurst commented Jul 29, 2016

ChristophWurst commented Jul 29, 2016

ChristophWurst commented Jul 29, 2016

jancborchardt commented Jul 30, 2016

ChristophWurst commented Aug 3, 2016

ChristophWurst commented Aug 3, 2016

jancborchardt commented Aug 3, 2016

jancborchardt commented Aug 3, 2016

tahaalibra commented Aug 3, 2016

ChristophWurst commented Aug 3, 2016

ChristophWurst commented Aug 3, 2016

jancborchardt commented Aug 3, 2016

tahaalibra commented Aug 3, 2016

jancborchardt commented Aug 4, 2016

ChristophWurst commented Aug 4, 2016

jancborchardt commented Aug 4, 2016

add aliases feature #1523

add aliases feature #1523

Conversation

tahaalibra commented Jun 8, 2016 • edited Loading

ChristophWurst commented Jun 8, 2016

ChristophWurst commented Jun 9, 2016 • edited by tahaalibra Loading

ChristophWurst commented Jun 10, 2016

Gomez commented Jun 10, 2016

tahaalibra commented Jun 10, 2016

ChristophWurst commented Jun 10, 2016

tahaalibra commented Jun 10, 2016

ChristophWurst commented Jun 10, 2016

ChristophWurst Jun 12, 2016

Choose a reason for hiding this comment

tahaalibra commented Jun 14, 2016 • edited Loading

tahaalibra commented Jun 18, 2016 • edited by Gomez Loading

Gomez commented Jul 22, 2016

ChristophWurst Jul 28, 2016

Choose a reason for hiding this comment

ChristophWurst commented Jul 29, 2016

ChristophWurst commented Jul 29, 2016

ChristophWurst commented Jul 29, 2016

jancborchardt commented Jul 30, 2016

ChristophWurst commented Aug 3, 2016

ChristophWurst commented Aug 3, 2016

jancborchardt commented Aug 3, 2016

jancborchardt commented Aug 3, 2016

tahaalibra commented Aug 3, 2016

ChristophWurst commented Aug 3, 2016

ChristophWurst commented Aug 3, 2016

jancborchardt commented Aug 3, 2016

tahaalibra commented Aug 3, 2016

jancborchardt commented Aug 4, 2016

ChristophWurst commented Aug 4, 2016

jancborchardt commented Aug 4, 2016

tahaalibra commented Jun 8, 2016 •

edited

Loading

ChristophWurst commented Jun 9, 2016 •

edited by tahaalibra

Loading

tahaalibra commented Jun 14, 2016 •

edited

Loading

tahaalibra commented Jun 18, 2016 •

edited by Gomez

Loading