Custom Public Chats #57

Mikunj · 2019-10-09T22:54:11Z

No description provided.

Minor refactor.

Added migrations.

…erver # Conflicts: # Signal.xcodeproj/project.pbxproj

SignalMessaging/profiles/OWSProfileManager.m

Mikunj · 2019-10-10T01:22:34Z

SignalServiceKit/src/Loki/API/Group Chat/LokiPublicChatManager.swift

+    @objc(addChatWithServer:channel:name:)
+    public func addChat(server: String, channel: UInt64, name: String) -> LokiGroupChat {
+        let chat = LokiGroupChat(channel: channel, server: server, displayName: name, isDeletable: true)
+        let model = TSGroupModel(title: chat.displayName, memberIds: [ourHexEncodedPublicKey!, chat.server], image: nil, groupId: chat.idAsData!)


ourHexEncodedPublicKey! might cause a crash

Mikunj · 2019-10-10T22:15:58Z

Signal/src/AppDelegate.m

+        NSString *userDefaultsKey = [@"isGroupChatSetUp." stringByAppendingString:chat.id];
+        BOOL isChatSetUp = [NSUserDefaults.standardUserDefaults boolForKey:userDefaultsKey];
+        if (!isChatSetUp || !chat.isDeletable) {
+            [LKPublicChatManager.shared addChatWithServer:chat.server channel:chat.channel name:chat.displayName];


Need to update the display name on the server

Signal/src/Loki/NewPublicChatVC.swift

Mikunj · 2019-10-14T21:26:14Z

Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m

-        } else {
-            self.moderatorIconImageView.hidden = YES;
-        }
+        BOOL isModerator = [LKGroupChatAPI isUserModerator:incomingMessage.authorId forGroup:groupChat.channel onServer:groupChat.server];


groupChat could be nil here

Not actually, right? Unless we made a mistake somewhere?

getGroupChatForThreadID: returns nil if a group chat object doesn't exist. This may be the case for private group chats.

Okay, yeah, it makes sense if you're thinking about private group chats already

We should get on the same page about public chats / private group chats / private chats

LokiGroupChat should really be LokiPublicChat then

I'll refactor this now so we don't get confused anymore

Mikunj · 2019-10-14T21:27:29Z

SignalServiceKit/src/Loki/API/Group Chat/LokiGroupChat.swift

-    @objc public var id: String { return "\(server).\(channel)" }
-    @objc public var idAsData: Data? { return id.data(using: .utf8) }
-
+    @objc public let id: String


Why can't we let id be a computed variable?

It's something that doesn't (and shouldn't change). By making it a let instead of a computed variable you reflect that. It's also slightly more efficient though in practice that's unlikely to matter

nielsandriesse · 2019-10-14T02:13:56Z

Signal/src/Loki/NewPublicChatVC.swift

+        navigationItem.leftBarButtonItem = UIBarButtonItem(barButtonSystemItem: .stop, target: self, action: #selector(close))
+        title = NSLocalizedString("Add Public Chat Server", comment: "")
+        // Separator
+        let separator = UIView()


This separator is never added as a subview

nielsandriesse · 2019-10-14T02:14:52Z

Signal/src/Loki/NewPublicChatVC.swift

+
+        // Stack view
+        let stackView = UIStackView(arrangedSubviews: [
+            serverUrlTextField,


An explainer label would be nice

nielsandriesse · 2019-10-14T02:19:57Z

Signal/src/Loki/NewPublicChatVC.swift

+        presentAlert(alert)
+    }
+
+    private func updateButton(enabled: Bool) {


This shouldn't be under the interaction mark

nielsandriesse · 2019-10-14T02:21:19Z

Signal/src/Loki/NewPublicChatVC.swift

+
+    private func updateButton(enabled: Bool) {
+        addButton.setEnabled(enabled)
+        addButton.setTitle(enabled ? NSLocalizedString("Add", comment: "") : NSLocalizedString("Connecting to server", comment: ""))


These need to be localized

nielsandriesse · 2019-10-14T02:24:30Z

Signal/src/Loki/NewPublicChatVC.swift

+        let addButtonFont = UIFont.ows_dynamicTypeBodyClamped.ows_mediumWeight()
+        let addButtonHeight = addButtonFont.pointSize * 48 / 17
+        let addButton = OWSFlatButton.button(title: NSLocalizedString("Add", comment: ""), font: addButtonFont, titleColor: .white, backgroundColor: .lokiGreen(), target: self, selector: #selector(handleNextButtonTapped))
+        addButton.autoSetDimension(.height, toSize: addButtonHeight)


Convention is to not set up constraints here. The reason is that not all constraints can be set up at this point (the view needs to be added as a subview before some of them can be created), so the idea is to wait for that and then set up everything.

nielsandriesse · 2019-10-14T05:24:38Z

SignalServiceKit/src/Loki/API/Group Chat/LokiGroupChat.swift

+        return chats
+    }
+
+    @objc public var id: String { return "\(server).\(channel)" }


You can just compute these once and make these fields constant

nielsandriesse · 2019-10-14T05:25:06Z

SignalServiceKit/src/Loki/API/Group Chat/LokiGroupChat.swift

+    }
+
+    @objc public var id: String { return "\(server).\(channel)" }
+    @objc public var idAsData: Data? { return id.data(using: .utf8) }


If you use a failable initializer you can reflect the fact that this should either succeed or the whole chat shouldn't exist

nielsandriesse · 2019-10-14T05:27:43Z

SignalServiceKit/src/Loki/API/Group Chat/LokiGroupChatPoller.swift

@@ -1,6 +1,6 @@

 @objc(LKGroupChatPoller)
-public final class GroupChatPoller : NSObject {
+public final class LokiGroupChatPoller : NSObject {


Let's either move both this and RSSFeedPoller to core or neither

nielsandriesse · 2019-10-14T22:00:23Z

SignalServiceKit/src/Loki/API/Group Chat/LokiGroupChat.swift

-    @objc public var id: String { return "\(server).\(channel)" }
-    @objc public var idAsData: Data? { return id.data(using: .utf8) }
-
+    @objc public let id: String


It's something that doesn't (and shouldn't change). By making it a let instead of a computed variable you reflect that. It's also slightly more efficient though in practice that's unlikely to matter

nielsandriesse · 2019-10-14T22:25:05Z

Signal/src/ViewControllers/ConversationView/Cells/OWSMessageCell.m

-        } else {
-            self.moderatorIconImageView.hidden = YES;
-        }
+        BOOL isModerator = [LKGroupChatAPI isUserModerator:incomingMessage.authorId forGroup:groupChat.channel onServer:groupChat.server];


Not actually, right? Unless we made a mistake somewhere?

nielsandriesse · 2019-10-14T23:35:50Z

SignalServiceKit/src/Loki/API/Public Chat/LokiPublicChatAPI.swift

        }
    }

+    public static func resetLastMessageCache(for group: UInt64, on server: String) {


This also clears the last deletion cache

nielsandriesse · 2019-10-14T23:46:02Z

Signal/src/AppDelegate.m

@@ -1620,8 +1595,7 @@ - (void)createRSSFeedPollersIfNeeded

 - (void)startGroupChatPollersIfNeeded
 {
-    [self createGroupChatPollersIfNeeded];
-    if (self.lokiPublicChatPoller != nil) { [self.lokiPublicChatPoller startIfNeeded]; }
+    [LKPublicChatManager.shared startPollersIfNeeded];


This makes the function redundant so let's remove it then

Mikunj added 5 commits October 9, 2019 11:01

Added storage functions.

3c7369f

Minor refactor.

Remove dependency on hard coded public chat values.

683a5c1

Added PublicChatManager.

d8d3328

Added migrations.

Added Public chat VC

28d5e9c

Merge commit '21aa2f646465094c82a1d0062aa74f4a482ca31b' into custom-s…

315ba3e

…erver # Conflicts: # Signal.xcodeproj/project.pbxproj

Mikunj commented Oct 10, 2019

View reviewed changes

SignalMessaging/profiles/OWSProfileManager.m Outdated Show resolved Hide resolved

Mikunj commented Oct 10, 2019

View reviewed changes

nielsandriesse added 6 commits October 14, 2019 12:55

Merge branch 'dev' into custom-server

86550c8

Fix build

ddc9ead

Clean up NewPublicChatVC

78ce3e2

Add missing messages fetch & display name update

0f5ac64

Update MentionCandidateSelectionView for group chat changes

1d8bb5f

Update MentionUtilities for group chat changes

294d44c

Mikunj commented Oct 14, 2019

View reviewed changes

Partially implement feedback

7b0e2d0

nielsandriesse force-pushed the custom-server branch from 578d733 to 7b0e2d0 Compare October 14, 2019 21:54

Fix build

48883bf

nielsandriesse force-pushed the custom-server branch from 8b6cd4d to 48883bf Compare October 14, 2019 22:23

nielsandriesse reviewed Oct 14, 2019

View reviewed changes

nielsandriesse added 2 commits October 15, 2019 09:43

Fix duplication

131c27d

Refactor

b4af9d1

nielsandriesse reviewed Oct 14, 2019

View reviewed changes

nielsandriesse added 4 commits October 15, 2019 10:50

Clean

8e5e113

Re-order files

0dbc309

Update Pods

b1b378e

Clean

bb9f59e

nielsandriesse changed the title ~~Custom Public Servers~~ Custom Public Chats Oct 15, 2019

nielsandriesse merged commit dcca0cc into dev Oct 15, 2019

nielsandriesse deleted the custom-server branch October 15, 2019 00:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom Public Chats #57

Custom Public Chats #57

Mikunj commented Oct 9, 2019

Mikunj Oct 10, 2019

Mikunj Oct 10, 2019

Mikunj Oct 14, 2019

nielsandriesse Oct 14, 2019

Mikunj Oct 14, 2019 •

edited

Loading

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

Mikunj Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

nielsandriesse Oct 14, 2019

Custom Public Chats #57

Custom Public Chats #57

Conversation

Mikunj commented Oct 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mikunj Oct 14, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mikunj Oct 14, 2019 •

edited

Loading