feat: Add pin and unpin methods to Message class by nik1062 · Pull Request #84 · Code-Society-Lab/matrixpy

nik1062 · 2026-07-01T16:34:46Z

Hi there, I have resolved the issue by implementing the pin() and unpin() methods on the Message class!

PenguinBoi12

Nice work getting pin/unpin functionally correct here, the approach is generally good. A just left few suggestions to tighten up the implementation.

nik1062 · 2026-07-01T17:30:54Z

Hi Simon, Thank you for the review and suggestions. I have addressed the feedback by refactoring the implementation: 1. Extracted the shared logic for fetching/parsing the pinned event list into the private helper _fetch_pinned(). 2. Simplified exception handling: allowed MatrixError raised by helper methods to propagate naturally, wrapping only room_put_state() inside try/except Exception. 3. Utilized early returns in both pin() and unpin() to reduce nesting and improve readability. All unit tests are passing and the code has been formatted with black. Let me know if everything looks good now! Best regards, Nikunj Kumar

…

On Wed, Jul 1, 2026 at 10:47 PM Simon Roy ***@***.***> wrote: ***@***.**** requested changes on this pull request. Nice work getting pin/unpin functionally correct here, the approach is generally good. A just left few suggestions to tighten up the implementation. ------------------------------ In matrix/message.py <#84 (comment)> : > @@ -168,3 +175,83 @@ async def oops(ctx: Context): ) except Exception as e: raise MatrixError(f"Failed to delete message: {e}") + + async def pin(self) -> None: + """Pin this message to the room. + + ## Example + ```python + @bot.command() + async def pin(ctx: Context): + await ctx.message.pin() + ``` + """ + try: pin() and unpin() both have the same ~15 lines to fetch and parse the current pinned list. Worth extracting into a private helper: async def _fetch_pinned(self) -> list[str]: """Fetch the current pinned event IDs for this room.""" response = await self.client.room_get_state_event( room_id=self.room.room_id, event_type="m.room.pinned_events", ) if isinstance(response, RoomGetStateEventResponse): return list(response.content.get("pinned", [])) if isinstance(response, RoomGetStateEventError) and response.status_code == "404": return [] if isinstance(response, RoomGetStateEventError): raise MatrixError(f"Failed to fetch pinned events: {response.message}") raise MatrixError("Unexpected response type when fetching pinned events") This allow to reuse the code in both methods and avoid unnecessary repetitions: async def pin(self) -> None: """Pin this message to the room.""" pinned = await self._fetch_pinned() if self.event_id in pinned: return pinned.append(self.event_id) ... ------------------------------ In matrix/message.py <#84 (comment)> : > @@ -168,3 +175,83 @@ async def oops(ctx: Context): ) except Exception as e: raise MatrixError(f"Failed to delete message: {e}") + + async def pin(self) -> None: The current try/except MatrixError raise followed by a broad except Exception is redundant, the except MatrixError raise only exists to counteract the catch-all below it. We only need a try/except around the one call that can actually raise a raw exception: room_put_state: async def pin(self) -> None: """Pin this message to the room.""" ... try: response = await self.client.room_put_state( room_id=self.room.room_id, event_type="m.room.pinned_events", content={"pinned": pinned}, ) except Exception as e: raise MatrixError(f"Failed to pin message: {e}") if isinstance(response, RoomPutStateError): raise MatrixError(f"Failed to pin message: {response.message}") ------------------------------ In matrix/message.py <#84 (comment)> : > + + if self.event_id not in pinned: + pinned.append(self.event_id) + put_response = await self.client.room_put_state( + room_id=self.room.room_id, + event_type="m.room.pinned_events", + content={"pinned": pinned}, + ) + if isinstance(put_response, RoomPutStateError): + raise MatrixError(f"Failed to pin message: {put_response.message}") + except MatrixError: + raise + except Exception as e: + raise MatrixError(f"Failed to pin message: {e}") + + async def unpin(self) -> None: Same comments as the pin method ------------------------------ In matrix/message.py <#84 (comment)> : > + response = await self.client.room_get_state_event( + room_id=self.room.room_id, + event_type="m.room.pinned_events", + ) + + if isinstance(response, RoomGetStateEventResponse): + pinned = list(response.content.get("pinned", [])) + elif isinstance(response, RoomGetStateEventError): + if response.status_code == "404": + pinned = [] + else: + raise MatrixError(f"Failed to fetch pinned events: {response.message}") + else: + raise MatrixError("Unexpected response type when fetching pinned events") + + if self.event_id not in pinned: I prefer having an early return through a positive conditionnal rather than adding another block of code if self.event_id in pinned: return ... — Reply to this email directly, view it on GitHub <#84?email_source=notifications&email_token=BKT7KXQZJYUJJ7AZGIDHUZT5CVBT5A5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINRRGEZTKOBQG4Z2M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#pullrequestreview-4611358073>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BKT7KXRBSNZUEXPVJF6WU3D5CVBT5AVCNFSNUABFKJSXA33TNF2G64TZHM4TQOJYGE3DMOBWHNEXG43VMU5TINZYG42DINJRGE22C5QC> . You are receiving this because you authored the thread.Message ID: ***@***.***>

PenguinBoi12

LGTM 🦭 I left a small nit I missed earlier, but otherwise it looks good to me. Thank you!

However the CI fails, Run those locally (from the project directory) and make sure everything passes:

Run tests: pytest
Run type check: mypy matrix
Run formatting: black .

PenguinBoi12 · 2026-07-01T18:22:32Z

+        )
+
+        if isinstance(response, RoomGetStateEventResponse):
+            return list(response.content["pinned"])


nit: return response.content.get("pinned", [])

@nik1062 I've seen that you changed it and it's good but you don't need to cast it. return response.content.get("pinned", []) should be enough. You only need to fix that and the PR is good to go.

PenguinBoi12

CI is not passing

Make sure all those passes:

Run tests: pytest
Run type check: mypy matrix
Run formatting: black .

nik1062 · 2026-07-02T17:40:25Z

Hi @PenguinBoi12,

Thanks for the review! I've addressed all the requested changes and verified everything locally (black, mypy, and pytest). Please let me know if there's anything else I should improve. Thanks!

nik1062 · 2026-07-03T04:16:17Z

Thanks for the tip!

…

On Fri, 3 Jul, 2026, 03:39 Simon Roy, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In matrix/message.py <#84 (comment)> : > @@ -174,3 +181,76 @@ async def oops(ctx: Context): ) except Exception as e: raise MatrixError(f"Failed to delete message: {e}") + + async def _fetch_pinned(self) -> list[str]: + response = await self.client.room_get_state_event( A new way to handle matrix calls has been added. Basically, the call would become: response = await matrix_call( self.client.room_get_state_event( room_id=self.room.room_id, event_type="m.room.pinned_events", ) error_message="Failed to fetch pinned events" ) And you should be able to remove all the try catch and raising. — Reply to this email directly, view it on GitHub <#84?email_source=notifications&email_token=BKT7KXTS6NX4MHLZ5NJVJBL5C3MTHA5CNFSNUABKM5UWIORPF5TWS5BNNB2WEL2QOVWGYUTFOF2WK43UKJSXM2LFO4XTINRSGE2DKMRYGM4KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#pullrequestreview-4621452838>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BKT7KXWNETG2T3647SF3KC35C3MTHAVCNFSNUABFKJSXA33TNF2G64TZHM4TQOJYGE3DMOBWHNEXG43VMU5TINZYG42DINJRGE22C5QC> . You are receiving this because you were mentioned.Message ID: ***@***.***>

PenguinBoi12 · 2026-07-03T06:15:22Z

Thanks for the tip!

You're welcome! Please make the changes to use matrix_call and remove the redundant exceptions. Once this is done the PR will be good to go.

Thank you @nik1062!

PenguinBoi12

You forgot to address the matrix_call comment.

nik1062 · 2026-07-04T08:58:55Z

Thanks for the feedback, @PenguinBoi12. I've refactored pin() and unpin() in
message.py to use matrix_call for the room_put_state calls, bringing them in line
with the rest of the codebase. The _fetch_pinned helper intentionally retains its
manual response handling, as a 404 from room_get_state_event represents a valid "no
pinned events" state rather than an error. Tests have been updated accordingly — all 295
passing.

PenguinBoi12 · 2026-07-04T09:12:44Z

Thanks for the feedback, @PenguinBoi12. I've refactored pin() and unpin() in
message.py to use matrix_call for the room_put_state calls, bringing them in line
with the rest of the codebase. The _fetch_pinned helper intentionally retains its
manual response handling, as a 404 from room_get_state_event represents a valid "no
pinned events" state rather than an error. Tests have been updated accordingly — all 295
passing.

Did you forget to push because I can't see those changes.

nik1062 · 2026-07-04T14:06:57Z

Sorry about that — the commit was local but not pushed. It's up now, you should be able to see the changes.

…

On Sat, Jul 4, 2026 at 2:43 PM Simon Roy ***@***.***> wrote: *PenguinBoi12* left a comment (Code-Society-Lab/matrixpy#84) <#84 (comment)> Thanks for the feedback, @PenguinBoi12 <https://github.com/PenguinBoi12>. I've refactored pin() and unpin() in message.py to use matrix_call for the room_put_state calls, bringing them in line with the rest of the codebase. The _fetch_pinned helper intentionally retains its manual response handling, as a 404 from room_get_state_event represents a valid "no pinned events" state rather than an error. Tests have been updated accordingly — all 295 passing. Did you forget to push because I can't see those changes. — Reply to this email directly, view it on GitHub <#84?email_source=notifications&email_token=BKT7KXWGOBGZOIW5Y3IBYW35DDDCFA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOBYGE2DEMZSHE22M4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4881423295>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BKT7KXU7B7DHEV23UFU53HD5DDDCFAVCNFSNUABFKJSXA33TNF2G64TZHM4TQOJYGE3DMOBWHNEXG43VMU5TINZYG42DINJRGE22C5QC> . You are receiving this because you were mentioned.Message ID: ***@***.***>

PenguinBoi12

The PR is almost ready to get approved. As I mentioned I did some research and there's a validation that you do in _fetch_pinned that will always raise an error. Fix that and it's good to go.

It's important to test with real data when possible, test, specially when mocked, can give false positive.

PenguinBoi12 · 2026-07-04T23:07:37Z

+        if isinstance(response, RoomGetStateEventResponse):
+            return list(response.content.get("pinned", []))
+        if isinstance(response, RoomGetStateEventError):
+            if response.status_code == "404":


So I did some research and testing and this won't work. matrix does not return "404" as status code, it returns a "M_NOT_FOUND". So you need to change if response.status_code == "M_NOT_FOUND":

The reason why your tests worked is because you mocked the response; it was a false positive.

Client-Server API spec — GET /rooms/{roomId}/state/{eventType}/{stateKey}

matrix-org/matrix-spec — data/api/client-server/rooms.yaml

Client-Server API spec — Standard error response

matrix-nio/matrix-nio — src/nio/responses.py, ErrorResponse

Same file, ErrorResponse.from_dict

PenguinBoi12

You're almost there, only one thing to fix and it's ready to be merge : )

feat: Add pin and unpin methods to Message class

97b30fa

PenguinBoi12 requested changes Jul 1, 2026

View reviewed changes

Comment thread matrix/message.py Outdated

Comment thread matrix/message.py

Comment thread matrix/message.py

Comment thread matrix/message.py Outdated

refactor: address review comments for pin/unpin methods

faf005c

PenguinBoi12 approved these changes Jul 1, 2026

View reviewed changes

PenguinBoi12 requested changes Jul 1, 2026

View reviewed changes

PenguinBoi12 added the feature A new feature label Jul 2, 2026

nit: use .get() with default and fix formatting

fe4f108

Merge branch 'main' into issue-67-pin-unpin

c5000bf

Merge upstream/main: resolve conflicts in message.py and test_message.py

85321cb

nik1062 requested a review from PenguinBoi12 July 4, 2026 06:44

PenguinBoi12 requested changes Jul 4, 2026

View reviewed changes

Refactor pin/unpin methods to use matrix_call for error handling

d38b6e6

nik1062 requested a review from PenguinBoi12 July 4, 2026 09:12

PenguinBoi12 approved these changes Jul 4, 2026

View reviewed changes

PenguinBoi12 requested changes Jul 4, 2026

View reviewed changes

PenguinBoi12 assigned nik1062 Jul 4, 2026

PenguinBoi12 linked an issue Jul 4, 2026 that may be closed by this pull request

Message: Add pin() and unpin() methods #67

Open

Uh oh!

Conversation

nik1062 commented Jul 1, 2026

Uh oh!

PenguinBoi12 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

nik1062 commented Jul 1, 2026 via email

Uh oh!

PenguinBoi12 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PenguinBoi12 Jul 1, 2026

Choose a reason for hiding this comment

Uh oh!

PenguinBoi12 Jul 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PenguinBoi12 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nik1062 commented Jul 2, 2026

Uh oh!

nik1062 commented Jul 3, 2026 via email

Uh oh!

PenguinBoi12 commented Jul 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PenguinBoi12 left a comment

Choose a reason for hiding this comment

Uh oh!

nik1062 commented Jul 4, 2026

Uh oh!

PenguinBoi12 commented Jul 4, 2026

Uh oh!

nik1062 commented Jul 4, 2026 via email

Uh oh!

PenguinBoi12 left a comment

Choose a reason for hiding this comment

Uh oh!

PenguinBoi12 Jul 4, 2026

Choose a reason for hiding this comment

Uh oh!

PenguinBoi12 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

PenguinBoi12 left a comment •

edited

Loading

PenguinBoi12 Jul 2, 2026 •

edited

Loading

PenguinBoi12 left a comment •

edited

Loading

PenguinBoi12 commented Jul 3, 2026 •

edited

Loading

PenguinBoi12 left a comment •

edited

Loading