-
Notifications
You must be signed in to change notification settings - Fork 10
T2751 Clicking on the compassion logo in the menu should redirect to dashboard #200
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
base: 14.0-MyCompassion2.0
Are you sure you want to change the base?
Conversation
Summary of ChangesHello @SamBachmann, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request aims to modify the default home page behavior, ensuring that users are directed to the appropriate page based on their authentication status. Specifically, it introduces a new Python controller to intercept requests to the root URL, with the immediate effect of redirecting all such requests to the login page. The ultimate goal is to send logged-in users to their dashboard and unauthenticated users to the login page, with this PR laying the groundwork for the latter. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new controller to redirect users from the home page, which is a good approach for this kind of logic in Odoo. However, the current implementation is inefficient for logged-in users as it causes a double redirect. I've left a comment with a suggestion to implement the redirection logic directly in the new controller, which will resolve the inefficiency and make the code cleaner. Regarding your question in the PR description, using a Python controller is definitely the right approach and good practice in Odoo for this kind of logic, much more so than using XML templates.
|
Uuups I forgot to run the pre-commit. Now it's done and it's ready for a review 😄 |
NoeBerdoz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your solution is valid, but aggressive, I would prefer
Apply suggestion from @NoeBerdoz Co-authored-by: NoeBerdoz <44669187+NoeBerdoz@users.noreply.github.com>
|
Got it ! I applied your suggestion. |
Co-authored-by: ecino <ecino@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request correctly implements the redirection from the home page. For logged-in users, it redirects to the dashboard, and for anonymous users, to the login page. Using a Python controller for this logic is a good and standard practice in Odoo, as it keeps the redirection logic cleanly on the server-side. I have a couple of suggestions for improvement regarding URL consistency and code organization. Overall, this is a good solution.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
After a careful review of the suggestion of Noé and Ema, I found that the code was now not correctly working. |
In this PR I redirected the home page (that were accessed either with clicking on compassion logo or logging out) to
Implementation :
I added a python controller to achieve that.
I first tried to achieve this using xml template inheritance. After spending quite some time on it I found python controller solution very convenient. Let me know if it's a good practice or whether it has to be done through xml :)