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

111 remove public private switch #166

Merged
merged 13 commits into from
Sep 14, 2022

Conversation

odelcroi
Copy link
Member

@odelcroi odelcroi commented Sep 12, 2022

What have changed

  • Remove public switch button when room is private
  • Remove private switch button when room is public
  • Remove spaces related switch buttons

Tests

  • add e2e tests
  • add component test : partially because I can't create a mock public room, it is only taken as "Invite"

@odelcroi odelcroi linked an issue Sep 12, 2022 that may be closed by this pull request
11 tasks
const today = new Date().toISOString().slice(0, 10).replace(/-/g, '');

beforeEach(() => {
cy.loginUser(homeserverUrl, email, password);
Copy link
Contributor

Choose a reason for hiding this comment

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

J'ai fait une version de cy.loginUser sans arguments, pour eviter d'avoir a specifier les defaults. Dans cette PR : #171
(approuuuuuve ma PRRRRR, viennnnns 🐍 )

@@ -35,6 +35,7 @@ getVar('E2E_TEST_USER_HOMESERVER_URL');
getVar('E2E_TEST_USER_HOMESERVER_SHORT');

export default defineConfig({
watchForFileChanges : false,
Copy link
Contributor

Choose a reason for hiding this comment

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

On veut ca ?

Copy link
Member Author

Choose a reason for hiding this comment

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

oui sinon, a chaque changement, il relance les tests c'est tres penible


afterEach(() => {
// todo logout, otherwise the login test is not reliable aby more.
// todo delete room, otherwise the test user will end up with a million identical rooms after a while.
Copy link
Contributor

Choose a reason for hiding this comment

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

I have a PR to add this : #170
(apprrrouuuuuuuuuuuuuuve ma PRRRRRRRRR 🐍 😵‍💫)

@@ -0,0 +1,51 @@
import Chainable = Cypress.Chainable;

export default class RoomUtils {
Copy link
Contributor

Choose a reason for hiding this comment

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

alternatively, you could use cy.createRoom (for better independence between tests) :

createRoom(options: ICreateRoomOpts): Chainable<string>;
/**

But if it works, fine, let's move forward.

//arrange
render(<TchapJoinRuleSettings {...props} />);

//assert that spaces option is not here while private and public are
Copy link
Contributor

Choose a reason for hiding this comment

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

public is asserted NOT there in the test. Is comment out of date ?

});


function openRoomAccessSettings(){
Copy link
Contributor

Choose a reason for hiding this comment

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

on met ca dans les utils peut etre ? Chepa

src/components/views/settings/TchapJoinRuleSettings.tsx Outdated Show resolved Hide resolved
</>,
}]; */

//tchap : we do not permit to change the type of room, thus display only one option
Copy link
Contributor

Choose a reason for hiding this comment

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

can you specify where tchap code ends, as well ? So that we know what's ours and what's not.

/**
* TCHAP : disable space-related options if create space feature is not enabled
*/
if (ComponentVisibilityCustomisations.shouldShowComponent(UIComponent.CreateSpaces)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

as we talked about, it would be better to create a separate flag, like showRooms or something

@odelcroi odelcroi merged commit 51a5ba9 into develop_tchap Sep 14, 2022
@odelcroi odelcroi linked an issue Sep 15, 2022 that may be closed by this pull request
@odelcroi odelcroi linked an issue Oct 18, 2022 that may be closed by this pull request
@odelcroi odelcroi deleted the 111-remove-public-private-switch branch March 31, 2023 15:53
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