[fractal/fix-join-room-alias] Fix join to room by alias




commit 0febc26f5392061e0f65597431ff851123926585
Author: Daniel García Moreno <dani danigm net>
Date:   Wed Aug 5 20:06:15 2020 +0200

    Fix join to room by alias
    
    The join_room method parameter was a RoomId so it was not possible to
    join to a room by alias. This patch changes that parameter to a
    RoomIdOrAliasId so we can use a RoomId or an AliasId.
    
    This patch also encode the `#` in the AliasId because in other case, the
    url is not correct and the request will fail.
    
    Besides the above fixes, this patch also improves a bit the error
    management for the join_room method, so the error dialog will have more
    information, for example if the room is not found on the server or
    something like that. This information is the direct response from the
    matrix server, but I think it's okay to show that to the user as
    feedback because in other case the user should run Fractal in debug mode
    to know what's happening.
    
    Fixes https://gitlab.gnome.org/GNOME/fractal/-/issues/658

 fractal-gtk/src/appop/invite.rs                    |  6 ++-
 fractal-gtk/src/appop/notify.rs                    |  7 ++++
 fractal-gtk/src/backend/room.rs                    | 47 ++++++++++++++++++----
 fractal-gtk/src/widgets/error_dialog.rs            |  5 ++-
 fractal-gtk/src/widgets/room.rs                    |  8 ++--
 .../src/r0/membership/join_room_by_id_or_alias.rs  | 13 ++++--
 6 files changed, 68 insertions(+), 18 deletions(-)
---
diff --git a/fractal-gtk/src/appop/invite.rs b/fractal-gtk/src/appop/invite.rs
index cf9f5ba2..9074b930 100644
--- a/fractal-gtk/src/appop/invite.rs
+++ b/fractal-gtk/src/appop/invite.rs
@@ -228,7 +228,11 @@ impl AppOp {
             let room_id = rid.clone();
             if accept {
                 thread::spawn(move || {
-                    match room::join_room(login_data.server_url, login_data.access_token, room_id) {
+                    match room::join_room(
+                        login_data.server_url,
+                        login_data.access_token,
+                        room_id.into(),
+                    ) {
                         Ok(jtr) => {
                             let jtr = Some(jtr);
                             APPOP!(set_join_to_room, (jtr));
diff --git a/fractal-gtk/src/appop/notify.rs b/fractal-gtk/src/appop/notify.rs
index 1a5a9e64..0cc66cf9 100644
--- a/fractal-gtk/src/appop/notify.rs
+++ b/fractal-gtk/src/appop/notify.rs
@@ -96,6 +96,13 @@ impl AppOp {
     pub fn show_error(&self, msg: String) {
         ErrorDialog::new(false, &msg);
     }
+
+    pub fn show_error_with_info(&self, msg: String, info: Option<String>) {
+        let dialog = ErrorDialog::new(false, &msg);
+        if let Some(text) = info {
+            dialog.set_property_secondary_text(Some(text.as_ref()));
+        }
+    }
 }
 
 fn dirty_truncate(s: &str, num_chars: usize) -> &str {
diff --git a/fractal-gtk/src/backend/room.rs b/fractal-gtk/src/backend/room.rs
index 1a17511f..c1ae07e5 100644
--- a/fractal-gtk/src/backend/room.rs
+++ b/fractal-gtk/src/backend/room.rs
@@ -1,8 +1,9 @@
 use log::error;
 use serde_json::json;
 
-use fractal_api::identifiers::{Error as IdError, EventId, RoomId, UserId};
+use fractal_api::identifiers::{Error as IdError, EventId, RoomId, RoomIdOrAliasId, UserId};
 use fractal_api::reqwest::Error as ReqwestError;
+use fractal_api::reqwest::StatusCode;
 use fractal_api::url::{ParseError as UrlError, Url};
 use std::fs;
 use std::io::Error as IoError;
@@ -18,6 +19,7 @@ use crate::actions::AppState;
 use crate::backend::HTTP_CLIENT;
 use crate::util::cache_dir_path;
 
+use crate::error::StandardErrorResponse;
 use crate::types::Member;
 use crate::types::Message;
 use crate::types::{Room, RoomMembership, RoomTag};
@@ -36,6 +38,7 @@ use fractal_api::r0::membership::invite_user::Body as InviteUserBody;
 use fractal_api::r0::membership::invite_user::Parameters as InviteUserParameters;
 use fractal_api::r0::membership::join_room_by_id_or_alias::request as join_room_req;
 use fractal_api::r0::membership::join_room_by_id_or_alias::Parameters as JoinRoomParameters;
+use fractal_api::r0::membership::join_room_by_id_or_alias::Response as JoinRoomResponse;
 use fractal_api::r0::membership::leave_room::request as leave_room_req;
 use fractal_api::r0::membership::leave_room::Parameters as LeaveRoomParameters;
 use fractal_api::r0::message::create_message_event::request as create_message_event;
@@ -376,24 +379,46 @@ pub fn redact_msg(
 }
 
 #[derive(Debug)]
-pub struct JoinRoomError(ReqwestError);
+pub enum JoinRoomError {
+    Request(ReqwestError),
+    Response(StandardErrorResponse),
+}
 
 impl From<ReqwestError> for JoinRoomError {
     fn from(err: ReqwestError) -> Self {
-        Self(err)
+        Self::Request(err)
+    }
+}
+
+impl From<StandardErrorResponse> for JoinRoomError {
+    fn from(err: StandardErrorResponse) -> Self {
+        Self::Response(err)
     }
 }
 
 impl HandleError for JoinRoomError {
     fn handle_error(&self) {
-        let err_str = format!("{:?}", self);
+        let err_str: String;
+        let info: Option<String>;
+
+        match self {
+            JoinRoomError::Request(error) => {
+                err_str = format!("{:?}", error);
+                info = None;
+            }
+            JoinRoomError::Response(error) => {
+                err_str = error.error.clone();
+                info = Some(error.error.clone());
+            }
+        };
+
         error!(
             "{}",
             remove_matrix_access_token_if_present(&err_str).unwrap_or(err_str)
         );
         let error = i18n("Can’t join the room, try again.").to_string();
         let state = AppState::NoRoom;
-        APPOP!(show_error, (error));
+        APPOP!(show_error_with_info, (error, info));
         APPOP!(set_state, (state));
     }
 }
@@ -401,7 +426,7 @@ impl HandleError for JoinRoomError {
 pub fn join_room(
     base: Url,
     access_token: AccessToken,
-    room_id: RoomId,
+    room_id: RoomIdOrAliasId,
 ) -> Result<RoomId, JoinRoomError> {
     let room_id_or_alias_id = room_id.clone().into();
 
@@ -411,9 +436,15 @@ pub fn join_room(
     };
 
     let request = join_room_req(base, &room_id_or_alias_id, &params)?;
-    HTTP_CLIENT.get_client().execute(request)?;
+    let response = HTTP_CLIENT.get_client().execute(request)?;
 
-    Ok(room_id)
+    if response.status() != StatusCode::OK {
+        let resp: StandardErrorResponse = response.json()?;
+        Err(JoinRoomError::Response(resp))
+    } else {
+        let resp: JoinRoomResponse = response.json()?;
+        Ok(resp.room_id)
+    }
 }
 
 #[derive(Debug)]
diff --git a/fractal-gtk/src/widgets/error_dialog.rs b/fractal-gtk/src/widgets/error_dialog.rs
index 90950dc7..3bb50792 100644
--- a/fractal-gtk/src/widgets/error_dialog.rs
+++ b/fractal-gtk/src/widgets/error_dialog.rs
@@ -3,7 +3,7 @@ use gtk::prelude::*;
 
 // Shows an error dialog, and if it's fatal it will quit the application once
 // the dialog is closed
-pub fn new(fatal: bool, text: &str) {
+pub fn new(fatal: bool, text: &str) -> gtk::MessageDialog {
     let app = gio::Application::get_default()
         .expect("No default application")
         .downcast::<gtk::Application>()
@@ -16,7 +16,6 @@ pub fn new(fatal: bool, text: &str) {
         gtk::ButtonsType::Ok,
         text,
     );
-
     let app_weak = app.downgrade();
     dialog.connect_response(move |dialog, _| {
         dialog.destroy();
@@ -28,4 +27,6 @@ pub fn new(fatal: bool, text: &str) {
 
     dialog.set_resizable(false);
     dialog.show_all();
+
+    dialog
 }
diff --git a/fractal-gtk/src/widgets/room.rs b/fractal-gtk/src/widgets/room.rs
index 531ef6d6..1fc34070 100644
--- a/fractal-gtk/src/widgets/room.rs
+++ b/fractal-gtk/src/widgets/room.rs
@@ -127,16 +127,16 @@ impl<'a> RoomBox<'a> {
                 let server_url = login_data.server_url.clone();
                 let access_token = login_data.access_token.clone();
                 let room_id = room_id.clone();
-                thread::spawn(
-                    move || match room::join_room(server_url, access_token, room_id) {
+                thread::spawn(move || {
+                    match room::join_room(server_url, access_token, room_id.into()) {
                         Ok(jtr) => {
                             let jtr = Some(jtr);
                             APPOP!(set_join_to_room, (jtr));
                             APPOP!(reload_rooms);
                         }
                         Err(err) => err.handle_error(),
-                    },
-                );
+                    }
+                });
             });
             join_button.set_property_width_request(JOIN_BUTTON_WIDTH);
 
diff --git a/fractal-matrix-api/src/r0/membership/join_room_by_id_or_alias.rs 
b/fractal-matrix-api/src/r0/membership/join_room_by_id_or_alias.rs
index 0142fd09..d0df557b 100644
--- a/fractal-matrix-api/src/r0/membership/join_room_by_id_or_alias.rs
+++ b/fractal-matrix-api/src/r0/membership/join_room_by_id_or_alias.rs
@@ -2,8 +2,8 @@ use crate::r0::AccessToken;
 use reqwest::blocking::Client;
 use reqwest::blocking::Request;
 use reqwest::Error;
-use ruma_identifiers::RoomIdOrAliasId;
-use serde::Serialize;
+use ruma_identifiers::{RoomId, RoomIdOrAliasId};
+use serde::{Deserialize, Serialize};
 use url::Host;
 use url::Url;
 
@@ -14,6 +14,11 @@ pub struct Parameters {
     pub server_name: Vec<Host>,
 }
 
+#[derive(Clone, Debug, Deserialize)]
+pub struct Response {
+    pub room_id: RoomId,
+}
+
 // TODO: Implement Body
 
 pub fn request(
@@ -21,8 +26,10 @@ pub fn request(
     room_id_or_alias: &RoomIdOrAliasId,
     params: &Parameters,
 ) -> Result<Request, Error> {
+    // econde # in the room id to allow alias
+    let encoded_id = room_id_or_alias.to_string().replace("#", "%23");
     let url = base
-        .join(&format!("_matrix/client/r0/join/{}", room_id_or_alias))
+        .join(&format!("_matrix/client/r0/join/{}", encoded_id))
         .expect("Malformed URL in join_room_by_id_or_alias");
 
     Client::new().post(url).query(params).build()


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]