Skip to content

Use the actual current period on the usage page #16474

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

Closed
wants to merge 1 commit into from

Conversation

bonysureliya
Copy link

@bonysureliya bonysureliya commented Feb 20, 2023

Description

from date = previous month date
to date = current date

Related Issue(s)

Fixes #15544

Release Notes

Use the actual current period when visiting the usage page

Build Options:

  • /werft with-github-actions
    Experimental feature to run the build with GitHub Actions (and not in Werft).
  • leeway-no-cache
    leeway-target=components:all
  • /werft no-test
    Run Leeway with --dont-test

Preview Environment Options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-gce-vm
    If enabled this will create the environment on GCE infra
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

@bonysureliya bonysureliya requested a review from a team February 20, 2023 09:13
@werft-gitpod-dev-com
Copy link

annotations in the pull request changed, but user is not allowed to start a job

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 20, 2023

/werft run with-preview=true

👍 started the job as gitpod-build-patch-1-fork.11
(with .werft/ from main)

@gtsiolis
Copy link
Contributor

gtsiolis commented Feb 20, 2023

/werft run with-clean-slate-deployment=true with-preview=true recreate-vm=true

👍 started the job as gitpod-build-patch-1-fork.12
(with .werft/ from main)

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-patch-1.4 because the annotations in the pull request description changed
(with .werft/ from main)

Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Thanks for contributing, @bonysureliya! 🍊

I've updated the PR description to link back to the relevant issue and include the deleted release notes section.

I couldn't test this in the preview environment as the build was failing[1]:

Failed to compile.

src/components/UsageView.tsx
Syntax error: 'finally' expected (57:8)
  1. Could you fix this—I'll retrigger the build once you push some changes.
  2. You'll also need to sign a CLA[1] once before merging your first contribution. Cc @meysholdt

bonysureliya added a commit to bonysureliya/gitpod that referenced this pull request Feb 21, 2023
gtsiolis pushed a commit to bonysureliya/gitpod that referenced this pull request Mar 3, 2023
@gtsiolis gtsiolis force-pushed the patch-1 branch 2 times, most recently from 4a6f82b to bf9efa4 Compare March 3, 2023 15:56
@roboquat roboquat added size/XS and removed size/S labels Mar 3, 2023
@gtsiolis
Copy link
Contributor

gtsiolis commented Mar 3, 2023

@bonysureliya Friendly reminder from #16474 (review):

You'll also need to sign a CLA[1] once before merging your first contribution. Cc @meysholdt

@gtsiolis
Copy link
Contributor

gtsiolis commented Mar 3, 2023

/werft run with-preview

👍 started the job as gitpod-build-patch-1-fork.13
(with .werft/ from main)

@meysholdt
Copy link
Member

Thank you, @bonysureliya, for signing our CLA!

@gtsiolis gtsiolis self-requested a review March 6, 2023 20:42
@gtsiolis
Copy link
Contributor

gtsiolis commented Mar 9, 2023

/werft run with-preview

👍 started the job as gitpod-build-patch-1-fork.14
(with .werft/ from main)

@gtsiolis gtsiolis changed the title Implemented the date feature Use the actual current period on the usage page Mar 9, 2023
@gtsiolis gtsiolis added do-not-merge/hold and removed do-not-merge/cla-pending CLA has not been signed labels Mar 9, 2023
@gtsiolis
Copy link
Contributor

@bonysureliya The usage page still defaults to Month 1st - Today.

You can check the preview environment below:

https://patch-1.preview.gitpod-dev.com/workspaces

Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

@bonysureliya I've added one comment above. This seems to behave similarly as the current state, ses #16474 (comment).

@stale
Copy link

stale bot commented Mar 23, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Mar 23, 2023
Comment on lines +44 to +47
const today = new Date();
const prevMonth = new Date(today.getFullYear(), today.getMonth() - 1, today.getDate());
const todayStr = today.toISOString().slice(0, 10);
const prevMonthStr = prevMonth.toISOString().slice(0, 10);
Copy link
Member

Choose a reason for hiding this comment

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

If I'm reading this correctly, this change would ignore the the URL hash parameter and would always set it to one month back. That's actually not the desired behaviour, as it prevents deep-linking of the current view in the URL.

Showing the last 1 month, could be the default behaviour when no hash is supplied, however, but the change as it stands does not do that. You likely want to move the handling into the !match case of this if.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks alot I was trying to find out what was causing the I have done the similar changes

@stale stale bot removed meta: stale This issue/PR is stale and will be closed soon labels Mar 27, 2023
Copy link
Author

@bonysureliya bonysureliya left a comment

Choose a reason for hiding this comment

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

Review of Date Changing Functionality , @easyCZ Helped where it was catching bug

@stale
Copy link

stale bot commented Apr 7, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label Apr 7, 2023
@gtsiolis
Copy link
Contributor

@bonysureliya Is this ready for another review? I'd love to get this merged and fix this annoying issue in #16474.

Could you also make sure this is up-to-date with the main branch?

Cc @easyCZ

@stale stale bot removed the meta: stale This issue/PR is stale and will be closed soon label Apr 11, 2023
@bonysureliya
Copy link
Author

Yes I will be completing the issue in one or two days.

@stale
Copy link

stale bot commented May 1, 2023

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the meta: stale This issue/PR is stale and will be closed soon label May 1, 2023
@stale stale bot closed this May 8, 2023
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.

Use the actual current period when visiting the usage page
5 participants