refactor: cleaner separation of getSellers* methods

[dohaki]

Description

This proposes a cleaner separation for the work done in #295 and #306.

The class CoreSDK should have as less as possible business logic and should just be a wrapper around the different module handlers, subgraph helpers, etc. I, therefore, moved them out into the account module.

I also think it is cleaner to leave the getSellerByAddress as it is to match only the configured operator, treasury, etc. address. And add a new method getSellersByAddressOrAuthToken that is more distinct.

@albertfolch-redeemeum I guess these are some breaking changes for the interface.

Blocked by #292 but can be reviewed already.

[levalleux-ludo]

@dohaki
Not sure I understand why "it is cleaner to leave the getSellerByAddress as it is to match only the configured operator, treasury, etc. address. And add a new method getSellersByAddressOrAuthToken that is more distinct."
IMHO the dApp needs a single service to find back the seller account from the address given by the wallet when he's connecting.
What I mean is: I think the dApp should always use getSellersByAddressOrAuthToken() and never use getSellerByAddress()
And, by extension, the other core-SDK clients too (but maybe I'm wrong).

The usecase I'm trying to explain (which is the usecase the dApp is facing to) is the one implemented in this test case:

core-components/e2e/tests/core-sdk.test.ts

Line 602 in b7d5342

xtest("getSellerByAddress() retrieve the seller using the auth token", async () => {

Note: this test has not been pushed yet because it requires a change at the contracts repo.

[levalleux-ludo]

Description

This proposes a cleaner separation for the work done in #295 and #306.

The class CoreSDK should have as less as possible business logic and should just be a wrapper around the different module handlers, subgraph helpers, etc. I, therefore, moved them out into the account module.

I also think it is cleaner to leave the getSellerByAddress as it is to match only the configured operator, treasury, etc. address. And add a new method getSellersByAddressOrAuthToken that is more distinct.

@albertfolch-redeemeum I guess these are some breaking changes for the interface.

Blocked by #292 but can be reviewed already.

The class CoreSDK should have as less as possible business logic and should just be a wrapper around the different module handlers, subgraph helpers, etc. I, therefore, moved them out into the account module.

However, I agree with "The class CoreSDK should have as less as possible business logic and should just be a wrapper around the different module handlers, subgraph helpers, etc. I, therefore, moved them out into the account module."

[dohaki]

@dohaki Not sure I understand why "it is cleaner to leave the getSellerByAddress as it is to match only the configured operator, treasury, etc. address. And add a new method getSellersByAddressOrAuthToken that is more distinct." IMHO the dApp needs a single service to find back the seller account from the address given by the wallet when he's connecting. What I mean is: I think the dApp should always use getSellersByAddressOrAuthToken() and never use getSellerByAddress() And, by extension, the other core-SDK clients too (but maybe I'm wrong).

The usecase I'm trying to explain (which is the usecase the dApp is facing to) is the one implemented in this test case:

core-components/e2e/tests/core-sdk.test.ts

Line 602 in b7d5342

xtest("getSellerByAddress() retrieve the seller using the auth token", async () => {

Note: this test has not been pushed yet because it requires a change at the contracts repo.

@levalleux-ludo I mean the dApp can always use getSellersByAddressOrAuthToken. But we can not assume that every other client would also have some kind of auth token configured no? I think it is just cleaner and more distinct to name the functions like this. Based on your argumentation the method getSellerByAuthToken would also not be needed then?

[levalleux-ludo]

@dohaki Not sure I understand why "it is cleaner to leave the getSellerByAddress as it is to match only the configured operator, treasury, etc. address. And add a new method getSellersByAddressOrAuthToken that is more distinct." IMHO the dApp needs a single service to find back the seller account from the address given by the wallet when he's connecting. What I mean is: I think the dApp should always use getSellersByAddressOrAuthToken() and never use getSellerByAddress() And, by extension, the other core-SDK clients too (but maybe I'm wrong).
The usecase I'm trying to explain (which is the usecase the dApp is facing to) is the one implemented in this test case:

core-components/e2e/tests/core-sdk.test.ts

Line 602 in b7d5342

xtest("getSellerByAddress() retrieve the seller using the auth token", async () => {

Note: this test has not been pushed yet because it requires a change at the contracts repo.

@levalleux-ludo I mean the dApp can always use getSellersByAddressOrAuthToken. But we can not assume that every other client would also have some kind of auth token configured no? I think it is just cleaner and more distinct to name the functions like this. Based on your argumentation the method getSellerByAuthToken would also not be needed then?

OK, I agree to keep the 2 methods.
And also keeping getSellerByAuthToken() as there are getSellerByOperator/Clerk/Admin/Treasury()
Important to note that, after this merge, the dApp should always use getSellersByAddressOrAuthToken() instead of getSellerByAddress()
^^ @dennisfurrer @RobertoMSousa @albertfolch-redeemeum @bigerjot

[RobertoMSousa]

Converted to draft because this PR can't be merged at the moment because it will impact the dApp
https://boson-protocol.slack.com/archives/C01R75F5HFA/p1665499683633289?thread_ts=1665480156.174159&cid=C01R75F5HFA