Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: slack-notify cli #51

Merged
merged 10 commits into from
Jan 25, 2024
+279 −53
Merged

fix: slack-notify cli #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5f53150
refactor: rename conversation to im
pm5 Aug 29, 2023
bd5f4ce
fix: add missing attributes to im
pm5 Aug 29, 2023
1351642
fix: add conversation_obj type
pm5 Aug 29, 2023
79ff195
fix: slack-notify cli
pm5 Aug 29, 2023
5a681f0
fix: test conversations_list
pm5 Aug 30, 2023
840f950
Fix conversation test
Leonidas-from-XIV Jan 25, 2024
8a66679
Reformat `conversations.json`
Leonidas-from-XIV Jan 25, 2024
f5a0d2e
Fix another test with invalid channel names
Leonidas-from-XIV Jan 25, 2024
ec4156c
Fix another unit test issue
Leonidas-from-XIV Jan 25, 2024
c4fe7b1
Fix all remaining tests
Leonidas-from-XIV Jan 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 44 additions & 17 deletions src/lib/slacko.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,16 @@ type channel = ChannelId of string | ChannelName of string

type conversation = string

type im = string

type user = UserId of string | UserName of string

type bot = BotId of string

type group = GroupId of string | GroupName of string

(* TODO: Sure about user? *)
type chat = Channel of channel | Im of conversation | User of user | Group of group
type chat = Channel of channel | Im of im | User of user | Group of group

type sort_criterion = Score | Timestamp

Expand All @@ -226,13 +228,17 @@ let channel_of_yojson = function
| `String x -> Ok (ChannelId x)
| _ -> Error "Couldn't parse channel type"

let conversation_of_yojson = function
| `String x -> Ok x
| _ -> Error "Couldn't parse conversation type"

let group_of_yojson = function
| `String x -> Ok (GroupId x)
| _ -> Error "Couldn't parse group type"

let conversation_of_yojson = function
let im_of_yojson = function
| `String x -> Ok x
| _ -> Error "Couldn't parse conversation type"
| _ -> Error "Couldn't parse im type"

type topic_obj = {
value: string;
Expand All @@ -248,6 +254,7 @@ type channel_obj = {
creator: user;
is_archived: bool;
is_general: bool;
name_normalized: string;
Copy link
Owner

Choose a reason for hiding this comment

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

I guess conversations.list returns a conversation and not a channel_obj, so this needs to be updated somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It returns "a list of limited channel-like conversation objects".

Copy link
Owner

Choose a reason for hiding this comment

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

In such case I suggest renaming it to conversation_obj since the Slack API calls it the "conversation type".

I think in this case the API breakage is happening anyway so might as well stick to the naming that Slack uses so it is easier to understand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like channels.* API was superseded along with im.* and others by conversations.*. However, there are still channel, group, im, mpim API object types, that are different from conversation type. The codebase also has a conversation type, which is not the type for the conversation API object. It seems instead of attempting a fix to the notify CLI tool now, we need to overhaul the underlying types first.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is now built on #52 which provides a new conversation_obj type. I'm not sure about the types of id this new API can have though.

is_member: bool;
members: user list;
topic: topic_obj;
Expand All @@ -259,6 +266,26 @@ type channel_obj = {
num_members: int option [@default None];
} [@@deriving of_yojson { strict = false }]

type conversation_obj = {
id: conversation;
name: string;
is_channel: bool;
created: Timestamp.t;
creator: user;
is_archived: bool;
is_general: bool;
name_normalized: string;
is_member: bool;
topic: topic_obj;
purpose: topic_obj;
last_read: Timestamp.t option [@default None];
latest: string option [@default None];
unread_count: int option [@default None];
unread_count_display: int option [@default None];
num_members: int option [@default None];
} [@@deriving of_yojson { strict = false }]


type user_obj = {
id: user;
name: string;
Expand Down Expand Up @@ -336,7 +363,7 @@ type file_obj = {
(*public_url_shared: ???;*)
channels: channel list;
groups: group list;
ims: conversation list;
ims: im list;
initial_comment: Yojson.Safe.t option [@default None];
num_stars: int option [@default None];
} [@@deriving of_yojson { strict = false }]
Expand Down Expand Up @@ -485,6 +512,8 @@ type im_obj = {
user: user;
created: Timestamp.t;
is_user_deleted: bool;
is_open: bool option [@default None];
last_read: Timestamp.t option [@default None];
unread_count: int option [@default None];
unread_count_display: int option [@default None];
} [@@deriving of_yojson { strict = false }]
Expand Down Expand Up @@ -719,9 +748,9 @@ let maybe fn = function
| None -> None

(* nonpublic types for conversion in list types *)
type channels_list_obj = {
channels: channel_obj list
} [@@deriving of_yojson]
type conversations_list_obj = {
channels: conversation_obj list
} [@@deriving of_yojson { strict = false }]

type users_list_obj = {
members: user_obj list
Expand All @@ -735,15 +764,15 @@ type im_list_obj = {
ims: im_obj list;
} [@@deriving of_yojson]

let channels_list ?exclude_archived session =
api_request "channels.list"
let conversations_list ?exclude_archived session =
api_request "conversations.list"
|> optionally_add "exclude_archived" @@ maybe string_of_bool @@ exclude_archived
|> query session
>|= function
| `Json_response d ->
(match d |> channels_list_obj_of_yojson with
(match d |> conversations_list_obj_of_yojson with
| Ok x -> `Success x.channels
| Error x -> `ParseFailure x)
| Error e -> `ParseFailure e)
| #parsed_auth_error as res -> res
| _ -> `Unknown_error

Expand Down Expand Up @@ -782,11 +811,9 @@ let lookupk session (listfn : 'a listfn) filterfn k =
let id_of_channel session = function
| ChannelId id -> Lwt.return @@ `Found id
| ChannelName name ->
let base = String.sub name 1 @@ String.length name - 1 in
lookupk session channels_list (fun (x:channel_obj) -> x.name = base) @@ function
lookupk session conversations_list (fun (x:conversation_obj) -> x.name = name || x.name_normalized = name) @@ function
| [] -> `Channel_not_found
| [{id = ChannelId s; _}] -> `Found s
| [_] -> failwith "Bad result from channel id lookup."
| [{id = s; _}] -> `Found s
| _::_::_ -> failwith "Too many results from channel id lookup."

(* like id_of_channel but does not resolve names to ids *)
Expand Down Expand Up @@ -883,8 +910,8 @@ let user_of_string s =
let group_of_string s =
if s.[0] = 'G' then GroupId s else GroupName s

(* TODO Create a conversation if conversation does not exist? *)
let conversation_of_string s =
(* TODO Create a im if im does not exist? *)
let im_of_string s =
if s.[0] = 'D' then s else failwith "Not an IM channel"

let translate_parsing_error = function
Expand Down
50 changes: 38 additions & 12 deletions src/lib/slacko.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,9 +279,12 @@ type message
channel id. *)
type channel

(** A type of an IM conversation *)
(** A channel-like container for a conversation used by the Conversations API. *)
type conversation

(** A type of an IM conversation *)
type im

(** An user, represented by either a user name or a user id. *)
type user

Expand All @@ -292,7 +295,7 @@ type bot
type group

(** A place one can post messages to. *)
type chat = Channel of channel | Im of conversation | User of user | Group of group
type chat = Channel of channel | Im of im | User of user | Group of group

(** What criterion to use in search. *)
type sort_criterion = Score | Timestamp
Expand Down Expand Up @@ -357,6 +360,7 @@ type channel_obj = {
creator: user;
is_archived: bool;
is_general: bool;
name_normalized: string;
is_member: bool;
members: user list;
topic: topic_obj;
Expand All @@ -368,6 +372,26 @@ type channel_obj = {
num_members: int option;
}

(** Object representing information about a Slack channel. *)
type conversation_obj = {
id: conversation;
name: string;
is_channel: bool;
created: timestamp;
creator: user;
is_archived: bool;
is_general: bool;
name_normalized: string;
is_member: bool;
topic: topic_obj;
purpose: topic_obj;
last_read: timestamp option;
latest: string option;
unread_count: int option;
unread_count_display: int option;
num_members: int option;
}

(** Object representing a message attachment field. *)
type field_obj = {
title: string option;
Expand Down Expand Up @@ -468,13 +492,15 @@ type groups_rename_obj = {
created: timestamp
}

(** Information about a direct conversation with a person. *)
(** Information about a direct im with a person. *)
type im_obj = {
id: string;
is_im: bool;
user: user;
created: timestamp;
is_user_deleted: bool;
is_open: bool option;
last_read: timestamp option;
unread_count: int option;
unread_count_display: int option;
}
Expand All @@ -483,7 +509,7 @@ type im_channel_obj = {
id: string;
}

(** Information about an direct conversation channel. *)
(** Information about an direct im channel. *)
type im_open_obj = {
no_op: bool option;
already_open: bool option;
Expand Down Expand Up @@ -555,7 +581,7 @@ type file_obj = {
(*public_url_shared: ???;*)
channels: channel list;
groups: group list;
ims: conversation list;
ims: im list;
initial_comment: Yojson.Safe.t option;
num_stars: int option;
}
Expand Down Expand Up @@ -690,9 +716,9 @@ val bot_of_string : string -> bot
id by means of an additional request. *)
val channel_of_string: string -> channel

(** Create a conversation type out of a given string. The string is usually
starting with a capital 'D' and represents an IM conversation channel. *)
val conversation_of_string: string -> conversation
(** Create a im type out of a given string. The string is usually
starting with a capital 'D' and represents an IM im channel. *)
val im_of_string: string -> im

(** {2 Slack API calls} *)

Expand Down Expand Up @@ -737,7 +763,7 @@ val channels_kick: session -> channel -> user -> [ `Success | parsed_auth_error
val channels_leave: session -> channel -> [ `Success of channel_leave_obj | parsed_auth_error | channel_error | archive_error | leave_general_error | `User_is_restricted | bot_error ] Lwt.t

(** Lists all channels in a Slack team. *)
val channels_list: ?exclude_archived:bool -> session -> [ `Success of channel_obj list | parsed_auth_error ] Lwt.t
val conversations_list: ?exclude_archived:bool -> session -> [ `Success of conversation_obj list | parsed_auth_error ] Lwt.t

(** Sets the read cursor in a channel. *)
val channels_mark: session -> channel -> timestamp -> [ `Success | parsed_auth_error | channel_error | archive_error | not_in_channel_error ] Lwt.t
Expand Down Expand Up @@ -823,16 +849,16 @@ val groups_set_topic: session -> group -> topic -> topic_result Lwt.t
val groups_unarchive: session -> group -> [ `Success | parsed_auth_error | channel_error | `Not_archived | `User_is_restricted | bot_error ] Lwt.t

(** Close a direct message channel. *)
val im_close: session -> conversation -> [ `Success of chat_close_obj | parsed_auth_error | channel_error | `User_does_not_own_channel ] Lwt.t
val im_close: session -> im -> [ `Success of chat_close_obj | parsed_auth_error | channel_error | `User_does_not_own_channel ] Lwt.t

(** Fetches history of messages and events from direct message channel. *)
val im_history: session -> ?latest:timestamp -> ?oldest:timestamp -> ?count:int -> ?inclusive:bool -> conversation -> history_result Lwt.t
val im_history: session -> ?latest:timestamp -> ?oldest:timestamp -> ?count:int -> ?inclusive:bool -> im -> history_result Lwt.t

(** Lists direct message channels for the calling user. *)
val im_list: session -> [ `Success of im_obj list | parsed_auth_error ] Lwt.t

(** Sets the read cursor in a direct message channel. *)
val im_mark: session -> conversation -> timestamp -> [ `Success | parsed_auth_error | channel_error | not_in_channel_error ] Lwt.t
val im_mark: session -> im -> timestamp -> [ `Success | parsed_auth_error | channel_error | not_in_channel_error ] Lwt.t

(** Opens a direct message channel. *)
val im_open: session -> user -> [ `Success of im_open_obj | parsed_auth_error | user_error | user_visibility_error ] Lwt.t
Expand Down
46 changes: 46 additions & 0 deletions test/abbrtypes.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,52 @@ let abbr_channel_obj (chan : Slacko.channel_obj) = {
type abbr_channel_obj_list = abbr_channel_obj list
[@@deriving show, yojson]

type abbr_conversation_obj = {
(* id: conversation; *)
name: string;
is_channel: bool;
created: Timestamp.t;
(* creator: user; *)
is_archived: bool;
is_general: bool;
is_member: bool;
(* members: user list; *)
topic: abbr_topic_obj;
purpose: abbr_topic_obj;
last_read: Timestamp.t option [@default None];
(* latest: json option [@default None]; *)
unread_count: int option [@default None];
unread_count_display: int option [@default None];
num_members: int option [@default None];
} [@@deriving show, yojson { strict = false }]

let abbr_conversation_obj (conversation : Slacko.conversation_obj) = {
name = conversation.Slacko.name;
is_channel = conversation.Slacko.is_channel;
created = conversation.Slacko.created;
is_archived = conversation.Slacko.is_archived;
is_general = conversation.Slacko.is_general;
is_member = conversation.Slacko.is_member;
topic = abbr_topic_obj conversation.Slacko.topic;
purpose = abbr_topic_obj conversation.Slacko.purpose;
last_read = conversation.Slacko.last_read;
unread_count = conversation.Slacko.unread_count;
unread_count_display = conversation.Slacko.unread_count_display;
num_members = conversation.Slacko.num_members;
}

type abbr_conversation_obj_list = abbr_conversation_obj list
[@@deriving show]

type abbr_conversation_list_obj = {
channels: abbr_conversation_obj list
} [@@deriving show, yojson { strict = false }]

let abbr_conversation_obj_list_of_yojson json =
match abbr_conversation_list_obj_of_yojson json with
| Ok obj -> Ok obj.channels
| (Error _) as err -> err

type abbr_message_obj = {
type': string [@key "type"];
ts: Timestamp.t;
Expand Down
Loading