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.

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

Draft - Feature #3: Database Redesign #5

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

klavenjones
Copy link
Owner

@klavenjones klavenjones commented Sep 22, 2021

When adding a feature for three or more users, I would update the conversation model. So let's say I wanted to have five people in each conversation. I would add multiple users to it like so:

Conversation.findConversation = async function (user1Id, user2Id) {
  const conversation = await Conversation.findOne({
    where: {
      user1Id: {
        [Op.or]: [user1Id, user2Id, user3Id, user4d, user5Id]
      },
      user2Id: {
        [Op.or]: [user1Id, user2Id, user3Id, user4d, user5Id]
      }, 
        user3Id: {
        [Op.or]: [user1Id, user2Id, user3Id, user4d, user5Id]
      },
      user4Id: {
        [Op.or]:[user1Id, user2Id, user3Id, user4d, user5Id]
      },
      user5Id: {
        [Op.or]:[user1Id, user2Id, user3Id, user4d, user5Id]
      } }
  });

  // return conversation or null if it doesn't exist
  return conversation;
};

After updating the model, I would then make the necessary associations between the User and Conversation entity.

Conversation.belongsTo(User, { as: "user1" });
Conversation.belongsTo(User, { as: "user2" });
Conversation.belongsTo(User, { as: "user3" });
Conversation.belongsTo(User, { as: "user4" });
Conversation.belongsTo(User, { as: "user5" });

Once done with updating the associations, we would have to make the necessary changes in the routing functions in conversations.js and a few other files in the code.

If our application is already deployed, I hope it's integrated with some form of CI/CD connected to GitHub. That way, we can create a separate development branch and work on this feature, and when we are done with creating the feature, we can make another branch called staging. This is where we test and review our application a bit more before we merge our branch to the main branch. Once the team reviews all the code, then we can merge, and once joined, the changes will automatically go live.

Update

Database Design

So after thinking it through, I think a better way to represent users and conversations in a database is to have a Many-to-Many relationship between both entities.

Users have many Conversations.

Conversations have many Users.

One way to achieve this would be to have a separate model or table (junction model) representing their relationship.

Instead of having a foreign key in each table, the junction model will use the IDs from the associated tables as a composite primary key.

ERD

So if I were to put this into practice, instead of updating the Conversations or Users Model, I would define a separate model called ConversationUsers:

const ConversationUsers = sequelize.define('ConversationUsers', {
  UserId: {
    type: DataTypes.INTEGER,
    references: {
      model: Users, 
      key: 'id'
    }
  },
  ConversationId: {
    type: DataTypes.INTEGER,
    references: {
      model: Conversations, 
      key: 'id'
    }
  }
});

//Then we can edit the current relationships like so. 
//We could've just done this and sequelize would've automatically generated a junction table, 
//but defining it ourselves gives us some advantages, like adding columns to our 
// junction table
User.belongsToMany(User, { through: ConversationUsers });
Conversation.belongsToMany(Conversation, { through: ConversationUsers });

Message.belongsTo(Conversation);
Conversation.hasMany(Message);

As far as the frontend UI design, assuming the conversations can have two or more participants. I would keep the same layout overall but tweak the chat sidebar and the active chat.

Here is what I would do on the front end.

  • Change the sidebar to show group conversations instead of the individual conversations associated with the current user.
    • Change the conversation list item design from individual conversations to group conversations. For example, for group conversations, instead of displaying the user name and avatar, like in the current conversation item, we can do a list item that displays the title of the group and a group avatar.
  • In the active chat, I would only make minor adjustments to reflect the extra users.
    • I'll keep the logged-in user displayed on the right side of the chat and everyone else on the left.
    • Maybe instead of an avatar indicating the last read message, we can use smaller check icons to indicate everyone who has read your last message. Another option is to remove the last read message indicators completely due to the number of users in the chat.
    • Maybe add a message to the group chat that will send a message to the chat to let users know when a person enters or leaves the chat.
  • I would Add UI controls for the add group chat functionality if we were to implement it.

One thing we can change is maybe adding an invite function to the UI, but that may be outside the scope of this ticket for now.

After changing the schema, I would have to update or add route handlers to reflect the schema.

Here is how I would build the routes:

api/conversations.js

	
//I think I would tweak this route a bit to get the conversations. 
// I would get all the conversations, the user is involved in. 
// Since the User and Conversations has a many to many relationships, between them,
//I can retrieve the user's convos with eager loading.

//GET
router.get("/", async (req, res, next) => {
  try {
    if (!req.user) {
      return res.sendStatus(401);
    }
    //Get User and all the conversations they are apart of.
    const user = await User.findOne({
      where: { id: req.user.id },
      attributes: ["id", "username", "photoUrl"],
      include: [
        {
          model: Conversation,
          as: "conversations",
          attributes: ["id"],
          order: [[Message, "createdAt", "ASC"]],
          required: false,
          include: [{ model: Message, order: ["createdAt", "ASC"] }]
        }
      ]
    });

    const conversations = user.conversations;
    conversations.reverse();

    for (let i = 0; i < conversations.length; i++) {
      //GET CONVO
      const convo = user.conversations[i];
      //Create JSON
      const convoJSON = convo.toJSON();

      //This will keep track of their online status. Instead of keeping track of other users,
      // I think keeping track of the current user's online status should be done here
      // Since there is more than two users to a group. There may be a way better way to achieve this.

      convoJSON.currentUser = user.toJSON();
      if (onlineUsers.includes(convoJSON.currentUser.id)) {
        convoJSON.currentUser.online = true;
      } else {
        convoJSON.currentUser.online = false;
      }

      const endOfConvoListIndex = convoJSON.messages.length - 1;
      convoJSON.latestMessageText =
        convoJSON.messages[endOfConvoListIndex].text;

      conversations[i] = convoJSON;
    }

    res.json(conversations);
  } catch (error) {
    next(error);
  }
});


//Create conversation
router.post("/", async (req, res, next) => {
  const { chatroom } = req.body;
  if (!req.user) {
    return res.sendStatus(401);
  }

  // Grab the user to so we can add to chatroom.
  const user = await User.findOne({
    where: { id: req.user.id },
    attributes: ["id", "username", "photoUrl"]
  });

  // We will create a group conversation
  const conversation = await Conversation.create({ name: chatroom });
  //Add the user who created it to the room automatically
  conversation.addUser(user);
  //Send Status Created
  res.status(201).json({ success: true });
});

api/messages.js

//I would also make some minor changes to the messages route, 
 

router.post("/", async (req, res, next) => {
  try {
    if (!req.user) {
      return res.sendStatus(401);
    }
    const senderId = req.user.id;
    //One thing to think about, is to get rid of the recipient ID,
    // due to the conversation having more then two users.
    const { text, conversationId, sender } = req.body;

    //Grab sender to check and see if they are in the conversation.
    const user = User.findOne({
      where: { id: senderId },
      attributes: ["id", "username", "photoUrl"]
    });

    // if we already know conversation id, we can save time and just add it to message and return
    if (conversationId) {
      //Grab the current conversation
      const conversation = await Conversation.findOne({
        where: { id: conversationId }
      });

      //Check to see if the user is in the convo.
      const inConvo = await converstion.hasUser(user);

      //If not send an error
      if (!inConvo) {
        return res.sendStatus(403);
      }

      //If the user is in the conversation, create message and send it add it to the conversation.
      const message = await Message.create({ senderId, text, conversationId });
      conversation.addMessage(message);

      return res.json({ message, sender });
    }

    // If we don't have conversation id, we need to make sure the conversation does not exist.
    let conversation = await Conversation.findAll();
    const convoExist = await user.hasConversation(conversation);
    //If it doesn't let's create a new one
    if (!convoExist) {
      // create conversation
      conversation = await Conversation.create({});
      if (onlineUsers.includes(sender.id)) {
        sender.online = true;
      }
    }

    const message = await Message.create({
      senderId,
      text,
      conversationId: conversation.id
    });

    res.json({ message, sender });
  } catch (error) {
    next(error);
  }
});

Sockets?

As for the sockets? Well, I think the only change I would make is creating a socket that fires when a user enters and leaves the group since we are now trying to support group conversations.

Redux State?

I would change the redux state to reflect the group conversation feature we just implemented on the backend on the client-side. I would remove the recipientId from the state due to having more than two users.

I would create a way to monitor an array of otherUsers, which would be an array of users who aren't the current logged-in user.

I would also make a thunkCreator function to create a new conversation. Those are the only changes that come to mind when redesigning this application.

Deployment?

When it comes to deploying the application, we can go in a few different directions. But doing a bit of research, I think it may be best to take a Blue/Green deployment pattern for this particular application. This pattern is when we can run two versions of the application. The existing version of the application is Blue, and the updated version of our application is the Green application. Both being active, but only one is public. (Typically the blue version). We can choose which version is live (blue version) with this approach while running tests on the other. (green version). Once we complete all the tests, we can then point all the traffic to the green version, and if anything goes wrong, we can switch back to the blue version.

When it comes to this app, we can have blue and green versions of both the frontend and the backend. We can use Docker and Kubernetes to isolate these versions, making it a bit easier to separate versions.

@1234tgk
Copy link
Collaborator

1234tgk commented Sep 22, 2021

Good work. I do have some problems with this design however:

If you update the conversation model such that it has from user1Id to user5Id, we would only have up to five people in a conversation. That is not what we want; we want theoretically unlimited number of people in our new conversation.

You might think then having an array of users inside the conversation model will work. However, this will cause some efficiency problem in case of looking up or deleting users are needed in a big group chat.

Can you think of some other way to design conversation with multiple users?

Also, I would love to hear some more details on these aspects after you remodel your conversation:

  1. How would you update the frontend UI for the new group chat feature?
  2. How would you update route handlers to reflect the schema change?
  3. How would you update socket event handlers to reflect the schema change?
  4. How would you update Redux state to reflect the schema change?
  5. I would also want more details on the deployment. Can you describe some steps to make the transition to new model on technical level?

Please append your answers to your existing draft before requesting for the second review 🙂

@klavenjones klavenjones reopened this Sep 23, 2021
@klavenjones
Copy link
Owner Author

@1234tgk Hmmm. So thinking about it, this would be a many-to-many relationship. A user belongs to many conversations, and a conversation can have many users. What if we create a table that kept track of their relationship? Where it would keep track of the userId and the conversationId?

@1234tgk
Copy link
Collaborator

1234tgk commented Sep 23, 2021

@klavenjones That sounds better. 👍 Try to elaborate more on the details on how the overall ERD would look like. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants