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

[muc] changeSubject errors should be propagated #596

Closed

Conversation

guusdk
Copy link
Member

@guusdk guusdk commented May 23, 2024

When Smack requests a subject change of a MUC, an error returned by the server (eg: 'forbidden') should be propagated (as suggested by the pre-existing javadoc).

When Smack requests a subject change of a MUC, an error returned by the server (eg: 'forbidden') should be propagated (as suggested by the pre-existing javadoc).
@guusdk
Copy link
Member Author

guusdk commented May 23, 2024

@guusdk guusdk requested a review from Flowdalic May 23, 2024 12:30
@Flowdalic
Copy link
Member

LGTM

But could you change the type of the argument of the StanzaIdFilter constructor from Stanza, to StanzaView? This would avoid introducing an new local variable stanza in the method and is something that we should do in any case (the stanza id is immutable, even in stanza builders). Bonus points if this is done in an individual commit.

@guusdk
Copy link
Member Author

guusdk commented May 23, 2024

I tried, but apparently the ID is not set yet without building the stanza.

guusdk added a commit to XMPP-Interop-Testing/smack-sint-server-extensions that referenced this pull request May 23, 2024
…bject" of XEP-0045: "Multi-User Chat"

Note that these tests will fail when using a version of Smack that does not fix a bug in its subject change implemetation. See igniterealtime/Smack#596
guusdk added a commit to XMPP-Interop-Testing/smack-sint-server-extensions that referenced this pull request May 23, 2024
…bject" of XEP-0045: "Multi-User Chat"

Note that these tests will fail when using a version of Smack that does not fix a bug in its subject change implementation. See igniterealtime/Smack#596
@Flowdalic
Copy link
Member

Fixed with 147071f

@Flowdalic Flowdalic closed this Jun 1, 2024
guusdk added a commit to XMPP-Interop-Testing/smack-sint-server-extensions that referenced this pull request Jun 14, 2024
…bject" of XEP-0045: "Multi-User Chat"

Note that these tests will fail when using a version of Smack that does not fix a bug in its subject change implementation. See igniterealtime/Smack#596
Fishbowler pushed a commit to XMPP-Interop-Testing/smack-sint-server-extensions that referenced this pull request Jun 20, 2024
…bject" of XEP-0045: "Multi-User Chat"

Note that these tests will fail when using a version of Smack that does not fix a bug in its subject change implementation. See igniterealtime/Smack#596
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants