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

Feature/1021 fix is campaign check #237

Merged

Conversation

tuj
Copy link
Contributor

@tuj tuj commented Mar 26, 2024

Link to ticket

https://leantime.itkdev.dk/#/tickets/showTicket/1021

Description

  • Fixed isCampaign check for playlist relation puts.
  • Removed redux/api.js since it was unused.
  • Removed js generated version of typescript api since it was unused.
  • Added skip to queries that rely on an argument to avoid undefined queries.

Checklist

  • My code is covered by test cases.
  • My code passes our test (all our tests).
  • My code passes our static analysis suite.
  • My code passes our continuous integration process.

@tuj tuj requested a review from rimi-itk March 26, 2024 08:15
@tuj tuj self-assigned this Mar 26, 2024
@tuj tuj added the bug Something isn't working label Mar 26, 2024
Copy link
Contributor

@rimi-itk rimi-itk left a comment

Choose a reason for hiding this comment

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

Looks good.

@@ -3,7 +3,7 @@ import { useTranslation } from "react-i18next";
import { useNavigate } from "react-router-dom";
import Form from "react-bootstrap/Form";
import { Button } from "react-bootstrap";
import { usePostV1UserActivationCodesActivateMutation } from "../../redux/api/api.generated";
import { usePostV1UserActivationCodesActivateMutation } from "../../redux/api/api.generated.ts";
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this (adding .ts) really change anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Otherwise it could not resolve the path.
The change is from using the .js file to using the .ts file (since I deleted the .js file).

page,
id,
},
{ skip: !id }
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not clear (to me) what the semantics of { skip: !id } is and what it's supposed to do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tuj tuj merged commit 82ff369 into os2display:develop Mar 26, 2024
4 checks passed
@tuj tuj deleted the feature/1021-fix-is-campaign-check branch March 26, 2024 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants