Skip to content

Add invite route. - #14

Open
Menrath wants to merge 1 commit into
moodlehq:mainfrom
Menrath:api_matrix_invite
Open

Add invite route.#14
Menrath wants to merge 1 commit into
moodlehq:mainfrom
Menrath:api_matrix_invite

Conversation

@Menrath

@Menrath Menrath commented Feb 19, 2025

Copy link
Copy Markdown

@andrewnicols andrewnicols left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically the API user needs to be a member of the room to do the invite, but I don't think that we currently have them in the virtual room.

This is something that we may need to add in future.

This change is mostly good otherwise aside from some coding style issues. The main thing that is missing is that it doesn't actually do what it needs to do - that is it does not invite the user into the room!

Comment thread application/src/Controller/MatrixController.php Outdated
Comment thread application/src/Controller/MatrixController.php Outdated
Comment thread application/src/Controller/MatrixController.php Outdated
Comment thread application/src/Controller/MatrixController.php Outdated
Comment thread application/src/Controller/MatrixController.php
Comment thread application/src/Controller/MatrixController.php Outdated
Comment thread application/src/Controller/MatrixController.php Outdated
@andrewnicols

Copy link
Copy Markdown
Contributor

Also can you please squash the commits. We don't need to break things apart - it serves no purpose or benefit, and can be confusing when tracking bugs.

@Menrath

Menrath commented Feb 20, 2025

Copy link
Copy Markdown
Author

@andrewnicols Thanks for reviewing this so quickly. I was not expecting this as there are very few guidelines for contributors. I also did not know what coding style to use, I could not find it explicitly stated in the repository. Adding that would help, and maybe I would have started contributing earlier! :)

I agree with your changes and will try to merge them in a minute. And of course I can squash them right away, unless you prefer to do a squash on the merge.

@Menrath Menrath closed this Feb 25, 2025
@Menrath Menrath reopened this Feb 25, 2025
Co-authored-by: Andrew Lyons <andrew@nicols.co.uk>

Actually store the status of the invited room member.
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