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

Allow intradef users to invite users from other homeserver in a private room #446

Conversation

aulamber
Copy link
Contributor

@aulamber aulamber commented Mar 2, 2023

Cf. issue #370

To reproduce the bugfix:

  1. Create a private room

Screenshot 2023-03-02 at 10 12 00

  1. Then add a user to this new private room. It works, because this PR changes the state of the room to federated by default.
    Cf. before there was a popin saying:

Screenshot 2023-03-02 at 10 15 37

@@ -35,7 +35,7 @@ export default class TchapUtils {
// Only show the federate switch to defense users : it's difficult to understand, so we avoid
// displaying it unless it's really necessary.
if (baseDomain === 'agent.intradef.tchap.gouv.fr') {
return { showRoomFederationOption: true, roomFederationDefault: false };
return { showRoomFederationOption: true, roomFederationDefault: true };
Copy link
Contributor

Choose a reason for hiding this comment

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

@aulamber In case of "forum" default is false, in case private room (external or not, it's always federation default)

Copy link
Contributor

@jdauphant jdauphant Mar 2, 2023

Choose a reason for hiding this comment

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

Maybe you can change the isFederation : this.setState({ tchapRoomType: tchapRoomType }); if the room type isn't public room, isFederated is set to true.

It will be better to enforce the federation parameter at moment of the room is creation (I haven't found were is the code of that).

@jdauphant
Copy link
Contributor

Close #370
^ this will close the linked issue on merge

@aulamber aulamber closed this Mar 3, 2023
@aulamber aulamber reopened this Mar 3, 2023
@aulamber aulamber requested a review from odelcroi March 3, 2023 13:49
private onCancel = () => {
this.props.onFinished(false);
};

private onFederatedChange = (isFederated: boolean) => {
this.setState({ isFederated: isFederated });
private onFederatedChange = (isForumFederated: boolean) => {
Copy link
Member

Choose a reason for hiding this comment

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

onForumFederatedChange

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified

const cli = MatrixClientPeg.get();
const baseDomain = cli.getDomain();

// Only show the federate switch to defense users : it's difficult to understand, so we avoid
// displaying it unless it's really necessary.
if (baseDomain === 'agent.intradef.tchap.gouv.fr') {
return { showRoomFederationOption: true, roomFederationDefault: false };
Copy link
Member

Choose a reason for hiding this comment

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

it should be : {showForumFederationSwitch: true, forumFederationSwitchDefaultValue: false}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified

Copy link
Contributor

@jdauphant jdauphant left a comment

Choose a reason for hiding this comment

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

@aulamber aulamber requested a review from NicolasBuquet March 8, 2023 11:05
@jdauphant
Copy link
Contributor

Je lance une review app pour tester

Copy link

@NicolasBuquet NicolasBuquet left a comment

Choose a reason for hiding this comment

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

I would rename method isSelectedRoomFederated in isSelectedRoomForumAndFederated

Copy link
Contributor

@jdauphant jdauphant left a comment

Choose a reason for hiding this comment

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

C'est bon, j'ai testé sur la review app sur l'instance intradef et interieur en créé des salons et des forums, ça a marché correctement

@jdauphant
Copy link
Contributor

@aulamber il manque quoi pour merger ?

@aulamber
Copy link
Contributor Author

isSelectedRoomFederated

En fait non, parce que ca peut etre n'importe quelle room qui peut etre federee, pas seulement le forum :)

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.

4 participants