Repository navigation
Conversation
|
Azure Static Web Apps: Your stage site is ready! Visit it here: https://witty-hill-08172a210-498.centralus.5.azurestaticapps.net |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80e44ec5cd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }; | ||
|
|
||
| const SBHInstanceLogin = ({ goBack, setRepoSelection }) => { | ||
| const SBHInstanceLogin = ({ opened = true, onClose, goBack, setRepoSelection }) => { |
There was a problem hiding this comment.
The newly accepted onClose callback is never invoked after a successful login; handleSubmit still unconditionally calls goBack(false). In importToStudy, where SBH_LOGIN is the initial modal, this closes the modal through an empty history without calling completeWorkflow, so callers such as ImportFile and DownloadMetadata wait forever on their Promise. In the standalone Cloud/Local modal, no goBack prop is supplied at all, so the call throws and the successful login is reported as a failure.
Useful? React with 👍 / 👎.
| const [nullSelected, setNullSelected] = useState(false); | ||
| const dispatch = useDispatch(); | ||
| const selected = useSelector(state => state.primaryRepository.sbhPrimary); | ||
| const setSelected = (value) => dispatch(setSBHPrimary(typeof value === 'function' ? value(selected) : value)); | ||
|
|
||
| const findInstance = (uri) => { | ||
| return instanceData.find((element) => element.registryURL === uri); | ||
| } | ||
| const selectedRepo = modalData.selectedRepo || selected; | ||
| const setSelected = (value) => { | ||
| const nextValue = typeof value === 'function' ? value(selected) : value; |
There was a problem hiding this comment.
When the unified create-study flow starts with an existing modalData.selectedRepo, this expression permanently prefers that initial value over the Redux selection. setSelected writes the new value at the outer modal-data level, while this component receives only modalData[currentModal], so changing or removing the repository leaves selectedRepo pointing at the original instance and the workflow verifies and creates against the wrong repository.
Useful? React with 👍 / 👎.
| <Button mr="md" onClick={() => { logout(selectedRepo, findInstance(selectedRepo)?.authtoken); }}> | ||
| Log Out | ||
| </Button> | ||
| <Button ml="auto" onClick={isUnifiedFlow ? handleUnifiedSelection : () => onClose?.()}> |
There was a problem hiding this comment.
For the non-unified SBHModal and LoginModal paths, selecting an instance with any stored token now immediately closes the modal. Previously this action called login, checked /profile, and cleared invalid credentials on failure; therefore expired or revoked tokens are now accepted and retained as a logged-in selection, causing later repository operations to fail without prompting the user to reauthenticate.
Useful? React with 👍 / 👎.
Gonza10V
left a comment
Gonza10V
left a comment
There was a problem hiding this comment.
The overall code looks good and my local deployment works as expected. There are some minor points raised by AI, please check them to see if they are pertinent.
|
Would be good to have maybe the username of email of the account connected to SBH when selecting instance. |
Kerem-G
left a comment
Kerem-G
left a comment
There was a problem hiding this comment.
I'm going to make the code changes I was planning on making in this branch in a separate pull request just focused on cleaning up the codebase. The code looked good for merge to me.
3 participants
This PR unifies the code for selecting SynBioHub and Flapjack instances. It also provides a means to register as a user on SynBioHub and Flapjack.
Closes #488
Closes #446