search

nodemailer / wildduck

Opinionated email server
https://wildduck.email/
European Union Public License 1.2
1.89k stars 263 forks source link

Main isn't required when updating an address #695

Closed louis-lau closed 3 months ago

NickOvt commented 3 months ago

Wondering whether that field is required at all in that endpoint 🤔. Because even at lines 1051-1056 we can omit main and find out that the address is main by previously comparing the given address path param and fetched user data. Something like

main = result.value.id === userData.address && userData.address === addressData.address
louis-lau commented 3 months ago

What do you mean exactly? You'd want to be able to set main: true in the update address endpoint right?

As far as I know it wasn't required before. Since I can only set it to true. It makes it impossible to update an address without making it the main address.

NickOvt commented 3 months ago

Yeah it doesn't need to be required indeed. I think I incorrectly analyzed the endpoint before. But what I was thinking with last comment is whether we need the main param at all? Like, the only values basically that it can get are undefined and true, because if it is false then we just stop and return an error (Cannot unset main status).

louis-lau commented 3 months ago

But what I was thinking with last comment is whether we need the main param at all

Otherwise how do I set an address as the main one?

NickOvt commented 3 months ago

What about lines 1051 - 1063? IF I understand correctly then the main value can actually be derived via an if statement. So basically if the path param id, which is the address, is equal to the email in the userData then it seems it is the main address, and if the new address is different then we set it as the new main.

But I guess actually having a field for main is clearer :D. So let's leave it like that right now

louis-lau commented 3 months ago

and if the new address is different then we set it as the new main.

Why? If I have a non-main alias I want to change to another address, I don't nessecarily want it to become my new main address.

This is an endpoint for updating an existing address.

louis-lau commented 3 months ago

And what if I had an existing alias that I wanted to make my main address? How could that be derived?

Sorry if I'm completely misunderstanding what you're saying

NickOvt commented 3 months ago

Right. It seems we need main for exactly this scenario. That we want to change a non-main address to be main. And that cannot be derived.

louis-lau commented 3 months ago

Yeah exactly haha. Phew, I thought I was really missing something for a second there

NickOvt commented 3 months ago

Yeah I suppose my brain was actively ignoring the last if (!main) statement haha. So I thought that it could be derived, but nope, we need it exactly for the reasons of setting a non-main address to main. So all good! :D :D