Date: Mon, 29 May 2023 11:40:02 -0700
From: William Casarin <jb55@jb55.com>
To: Ben Weeks <ben.weeks@outlook.com>
Cc: Bryan Montz <bryanmontz@me.com>, Terry Yiu <terryyiu@gmail.com>,  Joel
 Klabo <joelklabo@gmail.com>, Ben Weeks <ben.weeks@outlook.com>,  ericholguin
 <eric.holguinsanchez@gmail.com>, Swift <scoder1747@gmail.com>
Subject: Re: [PATCH damus] Add Recommended relays from the people you follow
 (ordered by popularity)
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset=utf-8
Tags: inbox

Hey Ben,

Comments inline. Lots of issues with this in its current state.

Also for everyone else on this email: I'm trying a new thing we're I'm
consolidating my comms through email to help with async dev until we
have a nostr-native solution.

I'll provide review from here from now on.

Cheers,


On Mon, May 29, 2023 at 02:00:20AM +0100, Ben Weeks wrote:
> ---
> struct RelayConfigView: View {
>     let state: DamusState
> +
>     @State var new_relay: String = ""
>     @State var relays: [RelayDescriptor]
>     @State private var showActionButtons = false
> 
> +    @StateObject var profile: ProfileModel
> +
>     @Environment(\.dismiss) var dismiss
> 
> -    init(state: DamusState) {
> +    init(state: DamusState, profile: ProfileModel) {
>         self.state = state
>         _relays = State(initialValue: state.pool.our_descriptors)
> +        self._profile = StateObject(wrappedValue: profile)

If you're passing in a profile model it should be an ObservedObject.
StateObjects are for View-owned state.

>     }
> 
> -    var recommended: [RelayDescriptor] {
> -        let rs: [RelayDescriptor] = []
> -        return BOOTSTRAP_RELAYS.reduce(into: rs) { xs, x in
> -            if state.pool.get_relay(x) == nil, let url = RelayURL(x) {
> -                xs.append(RelayDescriptor(url: url, info: .rw))
> +    var recommended: [(RelayDescriptor, Int)] {
> +        //var suggested_relays: [RelayDescriptor] = []
> +        var suggested_relays: [(RelayDescriptor, Int)] = [] // The Int is to keep count of how many people we follow us this relay.
> +
> +        if let profile_contacts = profile.contacts {

I'm not sure this is the way to do this. We should have a dedicated
class that collects this information in the DamusState, similar to what
we do with LikeCounter, etc. When processing contact lists, we can
update this state. Then we wouldn't need any special logic here.

> +            let contacts_pubkeys = profile_contacts.referenced_pubkeys.map { $0.ref_id }
> +            let following = FollowingModel(damus_state: state, contacts: contacts_pubkeys)
> +
> +            for pubkey in following.contacts {
> +                let contact = ProfileView(damus_state: state, pubkey: pubkey)

Constructing a view in a loop here just seems wrong...

> +                if let relays = contact.profile.relays {
> +                    for (urlString, relayInfo) in relays {
> +                        if let relayURL = RelayURL(urlString) {
> +                            let newRelay = RelayDescriptor(url: relayURL, info: relayInfo)
> +
> +                            if let index = suggested_relays.firstIndex(where: { $0.0.url == newRelay.url }) {
> +                                suggested_relays[index].1 += 1
> +                            } else {
> +                                suggested_relays.append((newRelay, 1))
> +                            }
> +                        }
> +                    }
> +                }
>             }
>         }