Skip to content

Conversation

@mlm483
Copy link
Contributor

@mlm483 mlm483 commented Jun 25, 2025

Description

Story: BI-2540

Most of the changes are renaming ORCID to OAuthID. Non-trivial changes are:

  • In UserService.java, biUser.setOauthProvider(oAuthProvider); sets the OAuth provider on the bi_user when the account is activated.
  • The migration, V1.34.0__rename-orcid.sql, renames orcid to oauth_id and adds a column oauth_provider that defaults to 'orcid'.
  • I added GitHub OAuth variables to .env.template and docker-compose.yml, you'll need to add and populate them in .env for testing.
  • In application.yml, I disabled flyway's out-of-order migrations, as this was causing some issues. I reverted this change, the pom.xml change below should make this unnecessary.
  • I also added configuration in pom.xml for the flyway maven plugin so that is knows to run both our SQL-based migrations and Java-based migrations. Because it's used in two places, I added the locations value to .env.template.

These changes go along with the bi-web PR for BI-2539, and this PR includes the changes from the bi-api PR for BI-2539.

bi-docker-stack PR: Breeding-Insight/bi-docker-stack#58

Dependencies

The feature/BI-2539 branch of bi-web, which adds UI elements to sign in with GitHub.

Testing

Make sure bi-web is running with VUE_APP_ALTERNATE_AUTHENTICATION_ENABLED=true.
Add the following variables for bi-api.

# GitHub OAuth variables. Only required if using GitHub as an alternative to ORCID.
GITHUB_OAUTH_CLIENT_ID=<Client ID of GitHub OAuth app.>
GITHUB_OAUTH_CLIENT_SECRET=<Client Secret of GitHub Oauth app.>

Try adding new users and opening the activation link (found in the email or debug logs) in a private window or separate browser. Test signing in with GitHub and ORCID.

Check the bi_user table, ensure that the oauth_provider column contains 'github' or 'orcid' as expected.

Checklist:

  • I have performed a self-review of my own code
  • I have tested my code and ensured it meets the acceptance criteria of the story
  • I have tested that my code works with both the brapi-java-server and BreedBase
  • I have create/modified unit tests to cover this change
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to documentation
  • I have run TAF: <please include a link to TAF run>

@mlm483 mlm483 marked this pull request as ready for review June 27, 2025 19:52
@mlm483 mlm483 requested review from a team, dmeidlin and nickpalladino and removed request for a team June 27, 2025 19:53
@mlm483 mlm483 requested a review from nickpalladino July 1, 2025 20:01
Copy link
Contributor

@dmeidlin dmeidlin left a comment

Choose a reason for hiding this comment

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

The Micronaut server fails to start up because the ProgramDAO is not recognized as a bean when the GermplasmDAO tries to inject it. Prior to this a maven clean install was done with the flyway migration, which correctly picked up the orcid_id to oauth_id column change in the bi_user table. After seeing the error, I wiped the bi_api docker volume as a precaution and tried again with a fresh install, same error with the Micronaut server. Also, the error goes away when reverting back to a different branch.

Not sure where the error lies, especially since neither ProgramDAOImpl.java class or ProgramDAO.java interface have been modified.

@mlm483
Copy link
Contributor Author

mlm483 commented Jul 7, 2025

The Micronaut server fails to start up because the ProgramDAO is not recognized as a bean when the GermplasmDAO tries to inject it. Prior to this a maven clean install was done with the flyway migration, which correctly picked up the orcid_id to oauth_id column change in the bi_user table. After seeing the error, I wiped the bi_api docker volume as a precaution and tried again with a fresh install, same error with the Micronaut server. Also, the error goes away when reverting back to a different branch.

Not sure where the error lies, especially since neither ProgramDAOImpl.java class or ProgramDAO.java interface have been modified.

Thanks for testing, I'll try to reproduce and troubleshoot this issue.

@mlm483
Copy link
Contributor Author

mlm483 commented Jul 10, 2025

@dmeidlin 0cf53b8 fixes an issue with migrations where the flyway maven plugin was not configured to find the Java-based migrations, this was causing various issues building the project.

Now that it's fixed, could you try building and running again?

Copy link
Contributor

@dmeidlin dmeidlin left a comment

Choose a reason for hiding this comment

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

With the change made to FLYWAY_LOCATIONS=filesystem:src/main/java/org/breedinginsight/db/migration,filesystem:src/main/resources/db/migration bi-api was able to compile after the flyway migrations. The login flow using Github via the Deltabreed UI works as expected.

@mlm483
Copy link
Contributor Author

mlm483 commented Aug 20, 2025

@mlm483 mlm483 merged commit fdb0f0e into develop Aug 20, 2025
@mlm483 mlm483 deleted the feature/BI-2540 branch August 20, 2025 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants