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.

Sign up for GitHub

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

Jump to bottom

[EEG] Electrodes query and file download fix #8478

Merged
merged 3 commits into from
Mar 28, 2023
Merged

[EEG] Electrodes query and file download fix #8478

merged 3 commits into from
Mar 28, 2023

Conversation

laemtl
Copy link
Contributor

@laemtl laemtl commented Mar 23, 2023 •
edited
Loading

PR #8242 modified the electrodes DB schemas and since then, the EEG Browser is broken.
This fixes the issue by rewriting the query logic.
Also fixes a few issues with the download buttons and the layout.

To test:

  • Make sure the EEG Browser loads
  • Create a new annotation
  • Check that the comment section of the new comment is displayed without any layout issues
  • Make sure the annotations, electrodes and all files are downloadable.

@laemtl laemtl requested a review from regisoc March 23, 2023 17:03
@laemtl laemtl added Category: Bug PR or issue that aims to report or fix a bug Critical to release PR or issue is key for the release to which it has been assigned labels Mar 23, 2023
@laemtl laemtl force-pushed the eeg-electrodes-query-fix branch from 50ce371 to 86ef06a Compare March 23, 2023 18:34
@regisoc
Copy link
Contributor

regisoc commented Mar 24, 2023

Hi thanks for this PR @laemtl I cannot interact with the Electrode download button right now.

image

@laemtl laemtl force-pushed the eeg-electrodes-query-fix branch from 86ef06a to e477e48 Compare March 24, 2023 15:19
@laemtl laemtl changed the title [EEG] Electrodes query fix [EEG] Electrodes query and download fix Mar 24, 2023
@laemtl laemtl changed the title [EEG] Electrodes query and download fix [EEG] Electrodes query and file download fix Mar 24, 2023
@laemtl
Copy link
Contributor Author

laemtl commented Mar 24, 2023 •
edited
Loading

Hi thanks for this PR @laemtl I cannot interact with the Electrode download button right now.

image

This is expected when no electrode file is found. Can you check if this is the case?

@laemtl laemtl force-pushed the eeg-electrodes-query-fix branch from e477e48 to bc5547e Compare March 24, 2023 15:40
regisoc
regisoc approved these changes Mar 24, 2023
View reviewed changes
Copy link
Contributor

@regisoc regisoc left a comment

Choose a reason for hiding this comment

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

LGTM

@laemtl laemtl force-pushed the eeg-electrodes-query-fix branch from b0e3815 to 6c33505 Compare March 27, 2023 14:43
@laemtl laemtl added this to the 25.0.0 milestone Mar 28, 2023
@laemtl laemtl removed the Critical to release PR or issue is key for the release to which it has been assigned label Mar 28, 2023
@driusan driusan merged commit 0fe247e into aces:main Mar 28, 2023
laemtl added a commit to jeffersoncasimir/Loris that referenced this pull request Mar 30, 2023
@laemtl
[EEG] Electrodes query and file download fix (aces#8478)
8a12eae 
PR aces#8242 modified the electrodes DB schemas and since then, the EEG Browser is broken.
This fixes the issue by rewriting the query logic.
Also fixes a few issues with the download buttons and the layout.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@regisoc regisoc regisoc approved these changes

Assignees

@regisoc regisoc

@laemtl laemtl

Labels
Category: Bug PR or issue that aims to report or fix a bug
Projects
None yet
Milestone
25.0.0
Development

Successfully merging this pull request may close these issues.
3 participants
@laemtl @regisoc @driusan