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

Override lemmy server URL when 'VITE__TEST_MODE' is set #1593

Merged
merged 2 commits into from
Aug 29, 2024

Conversation

aashu16
Copy link
Collaborator

@aashu16 aashu16 commented Aug 22, 2024

After the user logs in, state.auth.currentInstance changes from <hostname>:<PORT> to <hostname> [1]. Setting VITE__TEST_MODE=1 will override this behaviour with a couple of small changes and no side-effects. Using this var name because similar overrides might be needed elsewhere in the future and this should cover most (or all) of them.

Also updated the build step in e2e action.

[1]: Lemmy sets the iss claim to hostname when creating JWTs. authSlice.addJwt updates state.auth.currentInstance with the value of this claim.

* Change `urlSelector` and `updateConnectedInstance` to always use `getDefaultServer` as instance.
* In e2e.yml, include this in the build project step env.
@aashu16 aashu16 requested a review from aeharding as a code owner August 22, 2024 19:22
@aashu16
Copy link
Collaborator Author

aashu16 commented Aug 22, 2024

Missed an import. Added in the second commit.

export const urlSelector = (state: RootState) =>
instanceSelector(state) ?? state.auth.connectedInstance;
export const urlSelector = (state: RootState) => {
if (import.meta.env.VITE__TEST_MODE) return state.auth.connectedInstance;
Copy link
Contributor

Choose a reason for hiding this comment

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

curious: shouldn't this be implemented by mocking when running the tests? 🤔

Copy link
Owner

Choose a reason for hiding this comment

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

That would be a nice improvement

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sorry for taking this long to respond.

Not entirely clear on what we would want to mock here, @sharunkumar. Could you please expand on it? Feel free to chime in, @aeharding.

Copy link
Owner

@aeharding aeharding left a comment

Choose a reason for hiding this comment

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

Sorry for the delay! Need to get through all these PRs :)

@aeharding aeharding merged commit f8867f3 into main Aug 29, 2024
2 checks passed
@aeharding aeharding deleted the add-test-mode-flag branch August 29, 2024 04:18
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.

3 participants