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

53 - Add Login / Logout #67

Merged
merged 33 commits into from
May 10, 2023
Merged

53 - Add Login / Logout #67

merged 33 commits into from
May 10, 2023

Conversation

GPortas
Copy link
Contributor

@GPortas GPortas commented Apr 24, 2023

What this PR does / why we need it:

Adds the Login / Logout feature to the header component.

Log In and Sign Up redirects to JSF. To get the current authenticated user and Log Out, the application calls the js-dataverse package, which wraps those API operations.

Which issue(s) this PR closes:

Special notes for your reviewer:

This PR is part of an end-to-end flow that covers three PRs from three different repositories:

This branch includes a copy of the js-dataverse module with the latest changes (js-dataverse-2.0.0.tgz). As the module has not currently been released and therefore has not been pushed with these changes to npm either, it is the fastest way to have it available for development.

New GitHub environment variables required

The SPA now requires an .env file in order to be built, where the environment variable DATAVERSE_BACKEND_URL is specified. There are different points at which the SPA is built through GitHub actions and therefore require the setup of new secrets for such variable.

It is important that the variable is specified at the time of building, otherwise the application will fail once deployed. See:

throw Error('VITE_DATAVERSE_BACKEND_URL environment variable should be specified.')

e2e test action

This action requires that a new environment called "e2e" is created with a DATAVERSE_BACKEND_URL secret. At the moment, since e2e do not execute API calls, we can place a random URL. In the future we will have to specify a URL of a real instance for e2e testing.

deploy action

Within the "qa" environment, the DATAVERSE_BACKEND_URL secret must be also added, with the URL corresponding to the Dataverse instance to be used for QA. Keep in mind that to make API requests work the instance must be deployed with the changes developed in: GPortas:9531-session-api-auth-cors

Suggestions on how to test this:

Clone Dataverse repository pointing to branch GPortas:9531-session-api-auth-cors (IQSS/dataverse#9533)

Before building and deploying the Dataverse application locally, add the following properties to the microprofile-config.properties file:

dataverse.feature.api-session-auth=true

Once Dataverse is deployed and running on localhost:8080, run the SPA in development mode:

npm start

Does this PR introduce a user interface change? If mockups are available, please link/include them here:

logout_1

Is there a release notes update needed for this change?:

Additional documentation:

@GPortas
Copy link
Contributor Author

GPortas commented Apr 26, 2023

The e2e action is failing as the "e2e" environment and its DATAVERSE_BACKEND_URL secret are not defined in the repository configuration.

@MellyGray MellyGray changed the base branch from develop to feature/create-the-navigation-elements-of-the-design-system April 26, 2023 08:59
@pdurbin
Copy link
Member

pdurbin commented May 4, 2023

@GPortas can you please resolved the merge conflicts in this PR?

@pdurbin
Copy link
Member

pdurbin commented May 4, 2023

@GPortas it seems to work great in Firefox now! 🎉

I tested as of 454c7d4.

I installed LICEcap to record an animated gif:

login-logout

Before I pass this (and the other two pull requests) to QA can you please do the following?

  • Fix merge conflicts in this PR
  • Get the tests passing in this PR (all four are failing, currently)

Thanks!

(I'm aware that IQSS/dataverse#9533 has a failing test but it's the "push images to GHCR" thing we've been talking about, which is unrelated to the change in the PR.)

@GPortas
Copy link
Contributor Author

GPortas commented May 6, 2023

@pdurbin I just resolved the conflicts by updating with develop.

That animated gif is great! Although I think you're not testing something important, which is executing the log out from the SPA, not from JSF. That was the trigger for the issue we detected in Firefox.

Please, can you test that SPA log out works?

Thanks!

@pdurbin
Copy link
Member

pdurbin commented May 8, 2023

@GPortas sure, let me try again. Why is e2e push failing?

@pdurbin
Copy link
Member

pdurbin commented May 8, 2023

@GPortas

I'm not sure why but now I can't seem to log into the SPA at all. I built the frontend war file from ba8c085 and here it is if you'd like to take a look: dataverse-frontend.war.zip

A gif of the login attempts:

login-logout2

@GPortas
Copy link
Contributor Author

GPortas commented May 8, 2023

The e2e action is failing as the "e2e" environment and its DATAVERSE_BACKEND_URL secret are not defined in the repository configuration.

Hey @pdurbin, it will fail until we set the GitHub environment and a secret value. Thanks!

@pdurbin
Copy link
Member

pdurbin commented May 8, 2023

@GPortas right, right, I think we discussed the e2e test and I forgot to circle back and add the secrets for you. I'll bug you on Slack about this.

Meanwhile, I rebuilt the war file (stopped Storybook, removed the "dist" directory), and it works fine now. Sorry for the noise! Here's a gif showing how in Firefox you can logout from the SPA now:

login-logout3

I'm going to go ahead an move the whole lot (all three PRs) to QA. Thanks!

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

Logout is working on Firefox now. Approved!

@kcondon kcondon self-assigned this May 9, 2023
kcondon added a commit to kcondon/dataverse that referenced this pull request May 10, 2023
Allows testing of session based login/logout testing IQSS/dataverse-frontend#67
@kcondon
Copy link
Contributor

kcondon commented May 10, 2023

Issues found:

  1. When signing up from SPA, first attempt appears to do nothing, remains on signup page but throws server log null ptr. Clicking create again sends you to mydata page but logged in. If rather than clicking create again you went to homepage you would see the account was created successfully. See attached log for error:
    signup_err.txt
    [This was due to missing some url params]

  2. When refreshing the SPA homepage if not logged in, see a server log error. Note it also happens if you just type the url in the browser without hitting enter, I believe because it attempts to prefetch site icon? See attached log error:
    server_log_err_not_logged_in_spa.txt
    [This is expected because the existing api endpoint needs a valid user but will make a better endpoint later to fix]

@kcondon kcondon merged commit 112beb9 into develop May 10, 2023
@kcondon kcondon deleted the feature/53-login-logout branch May 10, 2023 19:04
jayanthkomarraju pushed a commit to jayanthkomarraju/dataverse-frontend that referenced this pull request May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Size: 3 A percentage of a sprint. 2.1 hours.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Login / Logout mechanism
5 participants