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] - Add export buttons to lists #193

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vikavorkin
Copy link

@vikavorkin vikavorkin commented Nov 11, 2023

Hi,
The default print QC codes dialog wasn't enough for me, trying to print labels using my brother label forces me to use their software.

The simplest thing was to export the data to a CSV and go from there.
Using the default export functionality of refinedev simply dumps the raw table data to a CSV file, this can be customized but it was enough for me.

The only issue I'm seeing is the QR code generation for those who may try to use the CSV, the code generates it as web+spoolman:s-${spool.id} but it's somewhat not known to the user.

Some pictures in dark and light modes.
image
image

@vikavorkin vikavorkin force-pushed the feature/export-table branch 2 times, most recently from 93141de to 6a18a87 Compare November 14, 2023 09:38
@vikavorkin
Copy link
Author

Re-done my commits to include the correct author name and email.

@vikavorkin
Copy link
Author

vikavorkin commented Nov 14, 2023

Example spools CSV exported from my instance

id registered first_used filament.id filament.registered filament.name filament.vendor.id filament.vendor.registered filament.vendor.name filament.material filament.price filament.density filament.diameter filament.weight filament.spool_weight filament.settings_extruder_temp filament.settings_bed_temp filament.color_hex remaining_weight used_weight remaining_length used_length archived
1 2023-11-03T11:51:39Z 2023-11-01T11:45:28.752000Z 1 2023-11-03T11:44:00Z Red PLA+ 1 2023-11-03T11:39:17Z Elegoo PLA+ 55.08 1.25 1.75 1000 171 215 60 FF0000 635 365 211201.85754300823 121399.49291842205 FALSE
2 2023-11-03T11:52:58Z 2023-11-03T15:07:58Z 1 2023-11-03T11:44:00Z Red PLA+ 1 2023-11-03T11:39:17Z Elegoo PLA+ 55.08 1.25 1.75 1000 171 215 60 FF0000 52.18969275155939 947.8103072484406 17358.362289335775 315242.9881720945 FALSE
3 2023-11-03T11:53:29Z undefined 2 2023-11-03T11:47:32Z Gray PLA+ 1 2023-11-03T11:39:17Z Elegoo PLA+ 51.5 1.25 1.75 1000 171 215 60 717171 961 39 319629.8977934345 12971.452667995782 FALSE
4 2023-11-03T11:54:16Z undefined 2 2023-11-03T11:47:32Z Gray PLA+ 1 2023-11-03T11:39:17Z Elegoo PLA+ 51.5 1.25 1.75 1000 171 215 60 717171 1000 0 332601.35046143027 0 FALSE

@Donkie
Copy link
Owner

Donkie commented Nov 25, 2023

This is really nice, something I've been thinking of doing myself.
Looks like a simple and clean implementation as well, great job :)

I noticed a few minor issues regarding missing columns:

  • The vendor comment seems to be missing when I export on the vendors page. However I see the filament.vendor.comment when I export from spool or filament page, not sure what's up with that
  • The filament comment is missing when I export from the spools page. However I see it when I export from the filaments page.

@vikavorkin
Copy link
Author

@Donkie After some investigation I found the issue regarding the missing columns.

The CSV export is outdated (uses a version from 5 years ago), it has a bug where the headers for the data come from the keys of the first object in the data array Source code of export-to-csv
This issue is fixed in the next bugfix version (commit)

I can't specify the headers manually currently, the API will not return non-set fields (it will return an empty string if you set and unset it).

Currently I have few options

  1. Wait for it to be fixed - opened a bug for refine to fix this - [BUG] - CSV Export uses an outdated pacakge - export-to-csv-fix-source-map refinedev/refine#5317
  2. Change the API to always include all the fields defined, even with null values.
  3. Look for a workaround on the client-side.

I will go with option 2 for now, I will try to see how seamless it can be (without affecting the DB).

@vikavorkin
Copy link
Author

@Donkie I saw that response_model_exclude_none is set for all request handler returning, un-setting it would resolve my issue.

Is there a reason why response_model_exclude_none is set?

@Donkie
Copy link
Owner

Donkie commented Nov 29, 2023

Looks like option 1 is on its way, so I think we can merge this and then update when its available.

Regarding your question, that was just a design choice I made, to discard nullable fields entirely if they are not set.

@vikavorkin
Copy link
Author

Going by your answer, after fixing the issue the excel will have empty values, for rows without a comment for ex.
Do you prefer an empty string to fill them or a null value?
By default empty cells are filled with null string
image

@Donkie
Copy link
Owner

Donkie commented Nov 29, 2023

Empty cell is more preferable imo

@Donkie
Copy link
Owner

Donkie commented Dec 17, 2023

@vikavorkin are you waiting for me to do something here to continue?

@vikavorkin
Copy link
Author

The refine fix is scheduled for January, not sure how Spoolman will handle the update, I guess it would be ok.

For now I can bring the CSV generation to Spoolman for now just to avoid the issues described, dropping it once refine's fix is released.

What you the think?

@Donkie
Copy link
Owner

Donkie commented Dec 18, 2023

I think it would be cool if you can get the CSV generation into Spoolman, then we're less dependent on refine

@vikavorkin
Copy link
Author

Hi @Donkie, sorry for the delay, didn't have much time for contribution.
I will try to have some basic export working at the weekend.
Sorry for keeping it hanging.

@Donkie
Copy link
Owner

Donkie commented Dec 26, 2023

No worries, take your time :)

@P-C-R
Copy link

P-C-R commented Feb 2, 2024

@vikavorkin any update on this? Would be nice if manufacturers of filament can share a whole database of filaments :-) best

@P-C-R
Copy link

P-C-R commented Feb 6, 2024

@vikavorkin or @Donkie what is missing for merge?

@vikavorkin
Copy link
Author

Hi @P-C-R
I didn't have time to work on this fix in the past couple of months, occupied with personal things.

From what I saw, refine released my fix to the export in https://github.com/refinedev/refine/releases/tag/%40refinedev/core%404.47.0
You can try and upgrade to it but this may have side effects.

Regarding the sharing of spool data by manufacturers, this is out of scope for this PR.
It would be awesome to have a global repository for available spools, closes thing I know of is the empty spool weight wiki.

@vikavorkin vikavorkin closed this Feb 8, 2024
@vikavorkin vikavorkin force-pushed the feature/export-table branch from 6dd5b6c to babb95d Compare February 8, 2024 16:43
@vikavorkin vikavorkin reopened this Feb 8, 2024
@StuSerious
Copy link
Contributor

StuSerious commented May 27, 2024

Regarding the sharing of spool data by manufacturers, this is out of scope for this PR. It would be awesome to have a global repository for available spools, closes thing I know of is the empty spool weight wiki.

@vikavorkin @P-C-R work has started on a unified filament database here: https://github.com/Donkie/SpoolmanDB

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.

4 participants