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

UI switched with a checkbox called Use Custom for Oracle #6696

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

radkmb
Copy link

@radkmb radkmb commented Jan 8, 2024

What type of PR is this?

  • Refactor
  • Feature

Description

Related to #6332 .
This change to the PR changed the UI so that the "Use Custom" checkbox switches the UI instead of the user having to type in a specific string to switch functions.

How is this tested?

  • Unit tests (pytest, jest)
  • E2E Tests (Cypress)
  • Manually
  • N/A

Related Tickets & Documents

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

2024-01-09.1.01.08.mov

@justinclift
Copy link
Member

@snickerjp This PR will probably be of interest to you. 😄

Copy link

codecov bot commented Jan 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4c47bef) 63.38% compared to head (3ada201) 63.38%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6696   +/-   ##
=======================================
  Coverage   63.38%   63.38%           
=======================================
  Files         162      162           
  Lines       13165    13165           
  Branches     1817     1817           
=======================================
  Hits         8344     8344           
  Misses       4532     4532           
  Partials      289      289           

@justinclift
Copy link
Member

@snickerjp Actually, would you be interested in reviewing this PR, and also seeing if the concept itself looks good? 😄

@snickerjp
Copy link
Member

Looks like a very good concept.
I too would have liked to make this change.

@justinclift
Copy link
Member

@snickerjp Ok, so should we merge this as-is? 😄

@snickerjp
Copy link
Member

@justinclift
Can I see some more?
I still don't understand the part where you pass values from Front UI to query_runner/oracle.py.

Maybe some changes are needed on the query_runner/oracle.py side.

@justinclift
Copy link
Member

@snickerjp The full source code for this PR is here:

https://github.com/radkmb/redash/tree/toggle-custom-ui

It looks like the current Redash master branch, with 3 new commits added on top:

https://github.com/radkmb/redash/commits/toggle-custom-ui/

You'll probably need to ask @radkmb about the changes, as I didn't write this code. 😄

@justinclift
Copy link
Member

@radkmb Hopefully you're ok to assist @snickerjp with this? 😄

@snickerjp
Copy link
Member

@justinclift
Thank you very much.

I will look into the part that works with query_runner/oracle.py.

@radkmb
Copy link
Author

radkmb commented Jan 9, 2024

@justinclift
Thanks for the confirmation!
I will also review the part that works with query_runner/oracle.py.

@snickerjp
Copy link
Member

With this source code, it doesn't seem to work without modifications

git checkout -b radkmb-toggle-custom-ui
git pull https://github.com/radkmb/redash.git toggle-custom-ui
yarn
make build
make compose_build
make up

The browser (or proxy) sent a request that this server could not understand.

edit-datasource-error

@justinclift
Copy link
Member

With this source code, it doesn't seem to work without modifications

Ahhh. Thanks for checking @snickerjp.

@radkmb Are you ok to look into the above error and fix things? 😄

@sk-on
Copy link

sk-on commented May 17, 2024

Why do I not have the option to connect to oracle Database

@justinclift
Copy link
Member

@sk-on Which version of Redash are you using? 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants