-
Notifications
You must be signed in to change notification settings - Fork 215
enhance: add dynamic filename for CSV exports based on filters #3043
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,7 @@ import { | |
| DokanModal, | ||
| } from '@dokan/components'; | ||
|
|
||
| import { Trash, ArrowDown, Home, Calendar, CreditCard } from 'lucide-react'; | ||
| import { Trash, ArrowDown, Home, Calendar, CreditCard, Loader2 } from 'lucide-react'; | ||
|
|
||
| // Define withdraw statuses for tab filtering | ||
| const WITHDRAW_STATUSES = [ | ||
|
|
@@ -88,6 +88,8 @@ const WithdrawPage = () => { | |
| approved: 0, | ||
| cancelled: 0, | ||
| } ); | ||
| const [ isExporting, setIsExporting ] = useState( false ); | ||
| const [ exportProgress, setExportProgress ] = useState( 0 ); | ||
| const [ filterArgs, setFilterArgs ] = useState( {} ); | ||
| const [ activeStatus, setActiveStatus ] = useState( 'pending' ); | ||
| const [ vendorFilter, setVendorFilter ] = useState< VendorSelect | null >( | ||
|
|
@@ -544,27 +546,72 @@ const WithdrawPage = () => { | |
| const tabsAdditionalContents = [ | ||
| <button | ||
| type="button" | ||
| className="inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-sm text-[#575757] hover:bg-[#7047EB] hover:text-white" | ||
| className="inline-flex items-center gap-2 rounded-md border border-gray-300 bg-white px-3 py-1.5 text-sm text-[#575757] hover:bg-[#7047EB] hover:text-white disabled:opacity-50 disabled:cursor-not-allowed" | ||
| disabled={ isExporting } | ||
| onClick={ async () => { | ||
| setIsExporting( true ); | ||
| setExportProgress( 0 ); | ||
| try { | ||
| // Minimal placeholder; backend export flow may vary. | ||
| // Attempt to hit export endpoint via same query params. | ||
| const path = addQueryArgs( 'dokan/v2/withdraw', { | ||
| ...view, | ||
| is_export: true, | ||
| } ); | ||
| const res = await apiFetch( { path } ); | ||
| if ( res && res.url ) { | ||
| window.location.assign( res.url as string ); | ||
| let page = 1; | ||
| let isComplete = false; | ||
| let downloadUrl = null; | ||
|
|
||
| // Poll until export is complete | ||
| while ( ! isComplete ) { | ||
| const path = addQueryArgs( 'dokan/v2/withdraw', { | ||
| per_page: 100, | ||
| page: page, | ||
| search: view?.search ?? '', | ||
| status: view?.status === 'all' ? '' : view?.status, | ||
| ...filterArgs, | ||
| is_export: true, | ||
| } ); | ||
|
|
||
| const res = await apiFetch( { path } ); | ||
|
|
||
| // Update progress if available | ||
| if ( res?.percentage !== undefined ) { | ||
| setExportProgress( res.percentage ); | ||
| } | ||
|
|
||
| if ( res?.step === 'done' ) { | ||
| isComplete = true; | ||
| downloadUrl = res.url; | ||
| } else if ( res?.step ) { | ||
| // Continue to next page | ||
| page = res.step; | ||
| } else { | ||
| throw new Error( 'Invalid response from export' ); | ||
| } | ||
| } | ||
|
|
||
| if ( downloadUrl ) { | ||
| window.location.assign( downloadUrl as string ); | ||
| } | ||
| } catch ( e ) { | ||
| // eslint-disable-next-line no-console | ||
| console.error( 'Export failed or not supported yet', e ); | ||
| alert( __( 'Export failed. Please try again.', 'dokan-lite' ) ); | ||
| } finally { | ||
| setIsExporting( false ); | ||
| setExportProgress( 0 ); | ||
| } | ||
| } } | ||
| > | ||
|
Comment on lines
547
to
600
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential memory leak: Add cleanup for unmounted component. The export loop (lines 560-586) continues making API calls until completion, but there's no mechanism to abort if the component unmounts during export. This can cause memory leaks and unnecessary API calls. Additionally, consider adding a maximum iteration limit as a safety mechanism to prevent infinite loops if the backend returns unexpected responses. 🔎 Suggested improvementsConsider using an onClick={ async () => {
setIsExporting( true );
setExportProgress( 0 );
+ const abortController = new AbortController();
+
+ // Cleanup function
+ const cleanup = () => {
+ abortController.abort();
+ setIsExporting( false );
+ setExportProgress( 0 );
+ };
+
try {
let page = 1;
let isComplete = false;
let downloadUrl = null;
+ const MAX_ITERATIONS = 1000; // Safety limit
+ let iterations = 0;
// Poll until export is complete
while ( ! isComplete ) {
+ if ( iterations++ >= MAX_ITERATIONS ) {
+ throw new Error( 'Export timeout: too many pages' );
+ }
+
const path = addQueryArgs( 'dokan/v2/withdraw', {
per_page: 100,
page: page,
search: view?.search ?? '',
status: view?.status === 'all' ? '' : view?.status,
...filterArgs,
is_export: true,
} );
- const res = await apiFetch( { path } );
+ const res = await apiFetch( {
+ path,
+ signal: abortController.signal
+ } );Then use a useEffect(() => {
// Return cleanup function that aborts export on unmount
return () => {
if (isExporting) {
// abort logic here
}
};
}, [isExporting]);
🤖 Prompt for AI Agents |
||
| <ArrowDown size={ 16 } /> | ||
| { __( 'Export', 'dokan-lite' ) } | ||
| { isExporting ? ( | ||
| <> | ||
| <Loader2 className="animate-spin" size={ 16 } /> | ||
| { exportProgress > 0 | ||
| ? __( `Exporting (${ exportProgress }%)`, 'dokan-lite' ) | ||
| : __( 'Exporting...', 'dokan-lite' ) | ||
| } | ||
| </> | ||
| ) : ( | ||
| <> | ||
| <ArrowDown size={ 16 } /> | ||
| { __( 'Export', 'dokan-lite' ) } | ||
| </> | ||
| ) } | ||
| </button>, | ||
| ]; | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix trailing whitespace.
Static analysis detected trailing whitespace on lines 121 and 126. Please remove to comply with coding standards.
🔎 Proposed fix
$store_name = ! empty( $store_info['store_name'] ) ? sanitize_title( $store_info['store_name'] ) : 'vendor-' . absint( $filters['user_id'] ); - + // Truncate if too long (max 50 chars for store name part). if ( strlen( $store_name ) > 50 ) { $store_name = substr( $store_name, 0, 44 ) . '-trunc'; } - + $parts[] = $store_name;Also applies to: 126-126
🧰 Tools
🪛 GitHub Check: Run PHPCS inspection
[failure] 121-121:
Whitespace found at end of line
🤖 Prompt for AI Agents