Skip to content

Next

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
    • Help
    • Support
    • Submit feedback
    • Contribute to GitLab
    • Switch to GitLab Next
  • Sign in / Register
Minds Mobile
Minds Mobile
  • Project
    • Project
    • Details
    • Activity
    • Releases
    • Cycle Analytics
    • Insights
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Charts
    • Locked Files
  • Issues 177
    • Issues 177
    • List
    • Boards
    • Labels
    • Service Desk
    • Milestones
  • Merge Requests 8
    • Merge Requests 8
  • Security & Compliance
    • Security & Compliance
    • Dependency List
  • Packages
    • Packages
    • List
    • Container Registry
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Charts
  • Create a new issue
  • Commits
  • Issue Boards
  • Minds
  • Minds MobileMinds Mobile
  • Merge Requests
  • !249

Closed
Opened 2 months ago by Ben Hayward@benhayward.ben
  • Report abuse
Report abuse

[Sprint/InterestingIguana](fix): Stopped rendering of undecrypted messages

Closes #887 (closed)

janky_messenger_decrypt

image

Edited 1 month ago by Ben Hayward

Check out, review, and merge locally

Step 1. Fetch and check out the branch for this merge request

-# All repo/branch refs have been quoted to allow support for special characters (such as #my-branch)
git fetch "https://gitlab.com/benhayward.ben/mobile-native.git" "fix/decrypted-chat-hide-1083"
git checkout -b "benhayward.ben/mobile-native-fix/decrypted-chat-hide-1083" FETCH_HEAD

Step 2. Review the changes locally

Step 3. Merge the branch and fix any conflicts that come up

git fetch origin
git checkout "origin/master"
git merge --no-ff "benhayward.ben/mobile-native-fix/decrypted-chat-hide-1083"

Step 4. Push the result of the merge to GitLab

git push origin "master"

Note that pushing to GitLab requires write access to this repository.

Tip: You can also checkout merge requests locally by following these guidelines.

Request to merge benhayward.ben:fix/decrypted-chat-hide-1083 into master
  • Email patches
  • Plain diff
Pipeline #74492449 passed for cd22d9a8 on benhayward.ben:fix/decrypted-chat-hide-1083
    Requires 2 more approvals.

    Closed by Ben Hayward 1 hour ago

    The changes were not merged into master

    Allows commits from members who can merge to the target branch

    Did not close #887 (closed)

    Deletes source branch

    • Discussion 12
    • Commits 6
    • Pipelines 5
    • Changes 3
    1/5 threads resolved
    • Loading...
    • Ben Hayward @benhayward.ben changed the description 2 months ago

      changed the description

    • Ben Hayward @benhayward.ben marked as a Work In Progress 2 months ago

      marked as a Work In Progress

    • Ben Hayward @benhayward.ben added 1 commit 2 months ago

      added 1 commit

      • aa55751d - Show a feint message when decrypt failed

      Compare with previous version

    • Ben Hayward @benhayward.ben changed the description 2 months ago

      changed the description

    • Brian Hatchet
      Brian Hatchet :speech_balloon: @brianhatchet · 2 months ago
      Developer

      Ok, I am going to need some help testing this one. Can you walk me through it?

    • Ben Hayward
      Ben Hayward @benhayward.ben · 2 months ago
      Developer

      Can do if you like when we get to it. Just restructuring this component a bit at the moment as per Martins specification in our chat.

    • Ben Hayward
      Ben Hayward @benhayward.ben · 2 months ago
      Developer

      The approvals aren't required for mobile-native but an extra pair of eyes is always welcomed.

    • Brian Hatchet :speech_balloon: @brianhatchet changed milestone to %"sprint: Jolly Jellyfish" 1 month ago

      changed milestone to %"sprint: Jolly Jellyfish"

    • Ben Hayward @benhayward.ben unmarked as a Work In Progress 1 month ago

      unmarked as a Work In Progress

    • Ben Hayward @benhayward.ben added MR::Awaiting Review Squad::Green scoped labels 1 month ago

      added MR::Awaiting Review Squad::Green scoped labels

      • Martin Santangelo
        Martin Santangelo @msantang78 · 1 month ago
        Maintainer
        Resolved by Ben Hayward 1 month ago

        Maybe we can move the decryption of the messages to the conversation store and run the decryption from the latest message to the oldest ones, once we hit the first decryption error we stop there and remove the other messages. In order to get the Messages re-rendered when the state change we should create a MessageModel with a decrypted observable

      • Brian Hatchet
        Brian Hatchet
        Ben Hayward
        Ben Hayward
        Last reply by Ben Hayward 1 month ago
    • Ben Hayward @benhayward.ben added MR::Requires Changes scoped label and automatically removed MR::Awaiting Review label 1 month ago

      added MR::Requires Changes scoped label and automatically removed MR::Awaiting Review label

    • Ben Hayward @benhayward.ben added 2 commits 1 month ago

      added 2 commits

      • 14d20b9a - Decryption working, save point commit
      • 827c30a3 - fixed things up a bit

      Compare with previous version

    • Ben Hayward @benhayward.ben added 1 commit 1 month ago

      added 1 commit

      • cd22d9a8 - formatting

      Compare with previous version

    • Ben Hayward @benhayward.ben resolved all threads 1 month ago

      resolved all threads

    • Ben Hayward @benhayward.ben added MR::Awaiting Review scoped label and automatically removed MR::Requires Changes label 1 month ago

      added MR::Awaiting Review scoped label and automatically removed MR::Requires Changes label

    • Martin Santangelo
      Martin Santangelo @msantang78 started a thread on the diff 1 month ago
      src/messenger/MessageModel.js
      5 export default class MessageModel extends BaseModel {
      6
      7 @observable decrypted = false;
      8 @observable friendly_ts = null;
      9 @observable guid = null;
      10 @observable message = null;
      11 @observable messages = null;
      12 @observable owner = null;
      13 @observable ownerObj = null;
      14 @observable owner_guid = null;
      15 @observable rowKey = null;
      16 @observable subtype = null;
      17 @observable time_created = null;
      18 @observable type = null;
      19 @observable __list = null;
      20 @observable decrypting = false;
      • Martin Santangelo
        Martin Santangelo @msantang78 · 1 month ago
        Maintainer

        Only decrypting and message should be observable. Also, I think that message doesn't have the property messages.

      • Please register or sign in to reply
    • Martin Santangelo
      Martin Santangelo @msantang78 started a thread on the diff 1 month ago
      src/messenger/conversation/Message.js
      24 24 import crypto from '../../common/services/crypto.service';
      25 25 import Tags from '../../common/components/Tags';
      26 26 import i18n from '../../common/services/i18n.service';
      27 import MessageModel from '../MessageModel';
      • Martin Santangelo
        Martin Santangelo @msantang78 · 1 month ago
        Maintainer

        this import is not needed

      • Please register or sign in to reply
    • Martin Santangelo
      Martin Santangelo @msantang78 started a thread on the diff 1 month ago
      src/messenger/conversation/Message.js
      55 .then(msg => {
      56 this.setState({ decrypted: true, msg });
      57 message.decrypted = true;
      58 message.message = msg;
      59 });
      60 } else {
      61 message.decrypted = true;
      62 message.message = '';
      63 this.setState({ decrypted: true, msg:'' });
      64 }
      65 }, 0);
      66
      67 } else {
      68 this.setState({ decrypted: true, msg: message.message });
      69 }
      44 this.message = this.props.message;
      • Martin Santangelo
        Martin Santangelo @msantang78 · 1 month ago
        Maintainer

        there is no need for componentWillMount here, we can access the prop this.props.message directly in the render method

        Edited by Martin Santangelo 1 month ago
      • Please register or sign in to reply
    • Martin Santangelo
      Martin Santangelo @msantang78 started a thread on the diff 1 month ago
      src/messenger/MessengerConversationStore.js
      61 62 this.moreData = false;
      62 63 }
      63 64
      65 conversation.messages = MessageModel.createMany(conversation.messages.reverse());
      66
      67 for (let i = 0; i < conversation.messages.length; i++) {
      68 conversation.messages[i].decrypt();
      • Martin Santangelo
        Martin Santangelo @msantang78 · 1 month ago
        Maintainer

        we should detect the decryption error and remove the older messages from there

      • Please register or sign in to reply
    • Ben Hayward @benhayward.ben added MR::Requires Changes scoped label and automatically removed MR::Awaiting Review label 3 weeks ago

      added MR::Requires Changes scoped label and automatically removed MR::Awaiting Review label

    • Ben Hayward
      Ben Hayward @benhayward.ben · 3 weeks ago
      Developer

      Had not seen your feedback here, my apologies Martin

    • Ben Hayward
      Ben Hayward @benhayward.ben · 1 week ago
      Developer

      @brianhatchet @msantang78 do you want me to take care of this still or is it left in your hands?

    • Ben Hayward @benhayward.ben closed 1 hour ago

      closed

    • You're only seeing other activity in the feed. To add a comment, switch to one of the following options.
    Please register or sign in to reply
    0 Assignees
    None
    Assign to
    (07/17) Jolly Jellyfish
    Milestone
    (07/17) Jolly Jellyfish
    Assign milestone
    None
    Time tracking
    No estimate or time spent
    2
    Labels
    MR::Requires Changes Squad::Green
    Assign labels
    • View project labels
    Lock merge request
    Unlocked
    3
    3 participants
    user avatar
    Ben Hayward
    user avatar
    Martin Santangelo
    user avatar
    Brian Hatchet
    Reference: minds/mobile-native!249