Skip to content

Fix matrix v3 attachments#1373

Merged
caronc merged 6 commits intocaronc:masterfrom
privacyfr3ak:fix_matrix_v3_attachments
Jul 29, 2025
Merged

Fix matrix v3 attachments#1373
caronc merged 6 commits intocaronc:masterfrom
privacyfr3ak:fix_matrix_v3_attachments

Conversation

@privacyfr3ak
Copy link
Contributor

@privacyfr3ak privacyfr3ak commented Jul 28, 2025

Description:

Related issue (if applicable): #

I noticed attachment was disabled and ignored on matrix v3 since it was broken in the upgrade to v3. I used the code commented with # FUTURE as a reference and tried to debug that. I noticed that the sending of mxc url was not upgraded to use PUT instead of POST. I also added the increment of the transaction ID after that part. Otherwise the rest of the message is ignored. I think I fixed the unit tests properly. I also did some local test with my tuwunel instance to send image and zip file without any problem with v3 enabled.

New Service Completion Status

  • apprise/plugins/.py
  • KEYWORDS
    • add new service into this file (alphabetically).
  • README.md
    • add entry for new service to table (as a quick reference)
  • packaging/redhat/python-apprise.spec
    • add new service into the %global common_description

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • No lint errors (use flake8)
  • 100% test coverage

Testing

Anyone can help test this source code as follows:

# Create a virtual environment to work in as follows:
python3 -m venv apprise

# Change into our new directory
cd apprise

# Activate our virtual environment
source bin/activate

# Install the branch
pip install git+https://github.com/privacyfr3ak/apprise.git@fix_matrix_v3_attachments

# Test out the changes with the following command:
apprise -t "Test Title" -b "Test Message" \
  -a path/to/image.jpg \
  matrixs://user:pass@hostname/#room_alias?v=3

@caronc
Copy link
Owner

caronc commented Jul 28, 2025

This is amazing; thank you so much; I just did a very, very, very large merge request. Hopefully your efforts aren't mucked up too much, but would you mind just rebasing (i see there are 2 conflicts). New updates allow you to do a lot more work with tox... (See #1368 for details)

I properly integrated ruff, and dropped setup.py. Since matrix was one of the 400+ files updated, you may just want to copy it out of the directory to be safe... then copy it back in and tox -e format with likely pretty closely align it again.

I'm sorry for the inconvienince; I do apprecate your PR.

@codecov
Copy link

codecov bot commented Jul 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.65%. Comparing base (70667a5) to head (de13ae4).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1373   +/-   ##
=======================================
  Coverage   99.65%   99.65%           
=======================================
  Files         170      170           
  Lines       21981    21989    +8     
  Branches     3467     3469    +2     
=======================================
+ Hits        21905    21913    +8     
  Misses         68       68           
  Partials        8        8           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@privacyfr3ak
Copy link
Contributor Author

I merged your last commit and managed the conflict. I did the formating and rerun the unit tests and local tests. Everything seems fine.

@caronc
Copy link
Owner

caronc commented Jul 29, 2025

Very weird that the build is failing on something you didn't even change:

  • .github/workflows/pkgbuild.yml (Line: 24, Col: 19): Unexpected value ''
image

I tried re-running it and it failed for the same reason. I'm going to chalk this up as a GitHub issue, but want to just wait an hour or so and try (the running of the workflow) again. Ideally I'd like the test the pass, but it's an odd-ball one so I won't hold back your great work otherwise.

Thanks again.

@privacyfr3ak
Copy link
Contributor Author

I'm not familiar with how the workflows works but it seems to fail because ${{ secrets.CR_PAT }} is equal to ''. I would suspect this variable is not available in the current context.

@caronc caronc merged commit f65f99c into caronc:master Jul 29, 2025
17 of 20 checks passed
@caronc
Copy link
Owner

caronc commented Jul 29, 2025

Merged! Thank you! 🚀

@caronc
Copy link
Owner

caronc commented Jul 29, 2025

I'm not familiar with how the workflows works but it seems to fail because ${{ secrets.CR_PAT }} is equal to ''. I would suspect this variable is not available in the current context.

Sorry @privacyfr3ak ; i didn't see your reply. I did fix this. It was exactly what you're eluding to... i'm good with code, but still learning workflows 🙂 . I didn't realize the key would only work for my commits and not others. I learned i could make the repo being referenced public (as there was nothing to hide anyway - didn't realize it was private) and drop the credentials entirely. Seems to be fine (for next time anyway we're good).

Thanks again for PR!

@privacyfr3ak
Copy link
Contributor Author

It has been a pleasure to contribute to this great project.

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