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

Lf 4015 hotfix routes #3081

Merged
merged 23 commits into from
Jan 17, 2024
Merged

Lf 4015 hotfix routes #3081

merged 23 commits into from
Jan 17, 2024

Conversation

Duncan-Brain
Copy link
Collaborator

Description
Fixing finances routes.

Jira link:

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Passes test case
  • UI components visually reviewed on desktop view
  • UI components visually reviewed on mobile view
  • Other (please explain)

Checklist:

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • The precommit and linting ran successfully
  • I have added or updated language tags for text that's part of the UI
  • I have added "MISSING" for all new language tags to languages I don't speak
  • I have added the GNU General Public License to all new files

@Duncan-Brain Duncan-Brain marked this pull request as ready for review January 17, 2024 19:40
@Duncan-Brain Duncan-Brain requested review from a team as code owners January 17, 2024 19:40
@Duncan-Brain Duncan-Brain requested review from antsgar and kathyavini and removed request for a team January 17, 2024 19:40
@@ -0,0 +1,60 @@
/*
* Copyright 2023 LiteFarm.org
Copy link
Collaborator

Choose a reason for hiding this comment

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

2024! 😆
Thank you for working on this, I will start testing now!

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm surprised we didn't have more of that these last two weeks!

@antsgar
Copy link
Collaborator

antsgar commented Jan 17, 2024

Looks good to me, at least on paper! I think we should merge it sooner rather than later to get more testing time on beta. Should we remove the deprecated routes file as part of this PR too?

antsgar
antsgar previously approved these changes Jan 17, 2024
@@ -901,7 +899,6 @@ const Routes = ({ isCompactSideMenu }) => {
/>
<Route path="/finances" component={Finances} />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@antsgar Just noticed.. is this missing a /finances/* like the other one in this file?

exact
component={ReadOnlyCustomRevenue}
/>
<Route
path="/finances/edit_custom_revenue/:revenue_type_id"
path={createEditCustomRevenueUrl(':revenue_type_id')}
exact
component={EditCustomRevenue}
/>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@antsgar The redirect below this line.. should it be redirecting from /finances to FINANCES_HOME_URL ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure honestly, I think I copied that from similar files

@kathyavini
Copy link
Collaborator

Should we remove the deprecated routes file as part of this PR too?

@antsgar I see it as deleted 🤔

@antsgar antsgar self-requested a review January 17, 2024 20:09
@kathyavini kathyavini merged commit cbc1bea into integration Jan 17, 2024
4 checks passed
antsgar pushed a commit that referenced this pull request Jan 17, 2024
@antsgar antsgar deleted the LF-4015-hotfix-routes branch January 26, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants