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

fix: Sorting & Display of FSE #1484

Merged
merged 2 commits into from
Dec 18, 2024
Merged

Conversation

dhaselhan
Copy link
Collaborator

  • Sort both paginated and non paginated by create date asc to match other schedules
  • Fix kWH usage column to use nice number formatter without decimals

* Sort both paginated and non paginated by create date asc to match other schedules
* Fix kWH usage column to use nice number formatter without decimals
@dhaselhan dhaselhan force-pushed the fix/daniel-fse-sorting-1460 branch from 742570a to ac1bc2d Compare December 17, 2024 22:54
Copy link

github-actions bot commented Dec 17, 2024

Frontend Test Results

  1 files  ±0  116 suites  ±0   39s ⏱️ -1s
390 tests ±0  367 ✅ ±0  20 💤 ±0  3 ❌ ±0 
392 runs  ±0  369 ✅ ±0  20 💤 ±0  3 ❌ ±0 

For more details on these failures, see this check.

Results for commit ac49b23. ± Comparison against base commit 5fa0ddc.

♻️ This comment has been updated with latest results.

Copy link

github-actions bot commented Dec 17, 2024

Backend Test Results

501 tests  ±0   501 ✅ ±0   1m 52s ⏱️ -1s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit ac49b23. ± Comparison against base commit 5fa0ddc.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@hamed-valiollahi hamed-valiollahi 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 to me!

As a suggestion, would sorting by id be more efficient here since integer comparisons are typically faster than timestamp comparisons like created_date?

@dhaselhan
Copy link
Collaborator Author

Looks good to me!

As a suggestion, would sorting by id be more efficient here since integer comparisons are typically faster than timestamp comparisons like created_date?

Depending on how ETL works this may or may not be true. ETL may insert records with a higher ID but a copied Create Date from TFRS. Ill ask Alex.

@hamed-valiollahi
Copy link
Collaborator

Looks good to me!
As a suggestion, would sorting by id be more efficient here since integer comparisons are typically faster than timestamp comparisons like created_date?

Depending on how ETL works this may or may not be true. ETL may insert records with a higher ID but a copied Create Date from TFRS. Ill ask Alex.

Thank you! In case there is significant improvement, we can also update the ETL to ensure the correct order of fetching data. Also, I noticed that there are a few places in the database where the tables don’t have proper indexing. It would be great if we could check and address that as well.

@dhaselhan
Copy link
Collaborator Author

Looks good to me!
As a suggestion, would sorting by id be more efficient here since integer comparisons are typically faster than timestamp comparisons like created_date?

Depending on how ETL works this may or may not be true. ETL may insert records with a higher ID but a copied Create Date from TFRS. Ill ask Alex.

Thank you! In case there is significant improvement, we can also update the ETL to ensure the correct order of fetching data. Also, I noticed that there are a few places in the database where the tables don’t have proper indexing. It would be great if we could check and address that as well.

Talked to Alex, he mentioned we are transferring Create Date so we can use that.

@dhaselhan dhaselhan merged commit 37360a9 into release-0.2.0 Dec 18, 2024
11 checks passed
@dhaselhan dhaselhan deleted the fix/daniel-fse-sorting-1460 branch December 18, 2024 17:09
@hamed-valiollahi
Copy link
Collaborator

Nice, thanks, Daniel!

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

Successfully merging this pull request may close these issues.

2 participants