Skip to content

Conversation

@SebastianWiz
Copy link
Contributor

Context

⛑️ Ticket(s): https://secure.helpscout.net/conversation/3049464511/88344?viewId=8172236

Summary

Issue: When users used the Rename Uploaded Files snippet on a multi-file upload field inside a Nested Form, filenames were correctly renamed when adding the child entry, but if the user then edited the child entry and saved, the filename became malformed.

This happened because GF's stored path metadata referenced the original URL instead of the URL of our renamed file. On edit, GF couldn't find the path info for the renamed URL and fell back to deriving a filename from the full URL, which after sanitization produced the long incorrect basename.

Fix: Immediately after each file is renamed, we now also write GF's file path meta for the new URL.

Quick before and after: https://www.loom.com/share/297e7afb604c4e9788ca24eb451b8fcf

…named multi-file uploads in Nested Forms became malformed after editing the child entry.
@coderabbitai
Copy link

coderabbitai bot commented Sep 8, 2025

Walkthrough

Update to gw-gravity-forms-rename-uploaded-files.php: version header bumped; rename handling changed to store renamed URL, preserve original path on rename failure, and on success synchronize Gravity Forms file-path metadata via GF_Field_FileUpload and gform_update_meta. No public API changes. (≤50 words)

Changes

Cohort / File(s) Summary
Gravity Forms rename/upload handling
gravity-forms/gw-gravity-forms-rename-uploaded-files.php
- Version header 2.6 → 2.7
- On rename success: assign URL to local $renamed_url, append it to results, and update GF file-path meta via GF_Field_FileUpload::get_upload_root_info() and gform_update_meta() with key from GF_Field_FileUpload::get_file_upload_path_meta_key_hash($renamed_url) storing path, url (trailing slashes), and file_name.
- On rename failure: append original $_file and continue (no metadata sync).
- Minor comment text change.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant GF as Gravity Forms
    participant PL as Plugin (rename_uploaded_files)
    participant FS as Filesystem
    participant GM as GF Meta

    U->>GF: Submit form with file upload
    GF->>PL: Invoke rename_uploaded_files(entry, form)
    PL->>FS: Attempt rename file
    alt rename succeeds
        FS-->>PL: Return renamed path
        PL->>PL: Build $renamed_url
        PL->>GF: GF_Field_FileUpload.get_upload_root_info(form.id)
        GF-->>PL: Return {root_path, root_url}
        PL->>GM: gform_update_meta(entry_id, meta_key_hash($renamed_url), {path, url, file_name})
        GM-->>PL: Meta updated
        PL-->>GF: Append $renamed_url to results
    else rename fails
        FS-->>PL: Error
        PL->>PL: Append original file path to results
    end
    PL-->>GF: Return updated file URLs list
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • veryspry
  • saifsultanc

Pre-merge checks (2 passed, 1 warning)

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title accurately summarizes the primary fix — preventing renamed multi-file uploads in Nested Forms from becoming malformed after editing a child entry — and aligns with the PR's change to write Gravity Forms' file-path metadata after renaming. It is specific and not misleading, though the leading filename prefix is unnecessary and makes the title slightly verbose.
Description Check ✅ Passed The description follows the repository template's Context and Summary sections, includes the HelpScout ticket link, a clear problem statement, root cause, the implemented fix (writing GF file-path meta after renaming), and a Loom demo link, giving reviewers sufficient context to understand and validate the change. This matches the raw summary and PR objectives and contains the critical information required for review.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch seb/fix/88344-nested-forms-edit-malformed

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Sep 8, 2025

Warnings
⚠️ When ready, don't forget to request reviews on this pull request from your fellow wizards.

Generated by 🚫 dangerJS against 705aa40

…named multi-file uploads in Nested Forms became malformed after editing the child entry.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
gravity-forms/gw-gravity-forms-rename-uploaded-files.php (2)

101-114: Solid fix syncing GF file-path meta; add guards and only sync on successful rename.
Great approach writing the path meta keyed by the renamed URL. Two robustness tweaks:

  • Only proceed if rename() succeeded.
  • Guard calls for older GF versions; skip meta sync if methods aren’t available.
  • Optional: remove stale meta for the original URL to avoid orphaned keys.

Apply within this hunk:

+				if ( ! $result ) {
+					$renamed_files[] = $_file;
+					continue;
+				}
+
-				$renamed_url = $this->get_url_by_path( $renamed_file, $form['id'] );
-				$renamed_files[] = $renamed_url;
+				$renamed_url = $this->get_url_by_path( $renamed_file, $form['id'] );
+				$renamed_files[] = $renamed_url;

-				// Keep Gravity Forms file-path meta in sync with the renamed URL.
-				$root = GF_Field_FileUpload::get_upload_root_info( $form['id'] );
-				gform_update_meta(
-					$entry['id'],
-					GF_Field_FileUpload::get_file_upload_path_meta_key_hash( $renamed_url ),
-					array(
-						'path'      => trailingslashit( $root['path'] ),
-						'url'       => trailingslashit( $root['url'] ),
-						'file_name' => wp_basename( $renamed_url ),
-					)
-				);
+				// Keep Gravity Forms file-path meta in sync with the renamed URL (GF 2.5+).
+				if (
+					class_exists( 'GF_Field_FileUpload' )
+					&& method_exists( 'GF_Field_FileUpload', 'get_upload_root_info' )
+					&& method_exists( 'GF_Field_FileUpload', 'get_file_upload_path_meta_key_hash' )
+				) {
+					$root = GF_Field_FileUpload::get_upload_root_info( $form['id'] );
+					gform_update_meta(
+						$entry['id'],
+						GF_Field_FileUpload::get_file_upload_path_meta_key_hash( $renamed_url ),
+						array(
+							'path'      => trailingslashit( $root['path'] ),
+							'url'       => trailingslashit( $root['url'] ),
+							'file_name' => wp_basename( $renamed_url ),
+						)
+					);
+					// Optional: clean up stale meta for the old URL.
+					if ( function_exists( 'gform_delete_meta' ) ) {
+						gform_delete_meta( $entry['id'], GF_Field_FileUpload::get_file_upload_path_meta_key_hash( $_file ) );
+					}
+				}

Quick checks to verify:

  • Edit a Nested Form child entry that has multi-file uploads; confirm file names remain correct.
  • Spot-check on a site with older GF (<2.5) if you support it; ensure no fatal due to missing methods.

117-117: Fix typo in comment.
Change “nfo images” → “no images”.

-			// In cases where 3rd party add-ons offload the image to a remote location, nfo images can be renamed.
+			// In cases where 3rd party add-ons offload the image to a remote location, no images can be renamed.
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e5ae702 and 19305b2.

📒 Files selected for processing (1)
  • gravity-forms/gw-gravity-forms-rename-uploaded-files.php (2 hunks)
🔇 Additional comments (1)
gravity-forms/gw-gravity-forms-rename-uploaded-files.php (1)

16-16: Version bump to 2.7 — looks good.
No concerns from me on the header update.

Copy link
Contributor

@saifsultanc saifsultanc left a comment

Choose a reason for hiding this comment

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

This looks good to me. I do agree with the coderabbit suggestion of the safety check for Only proceed if rename() succeeded.

Can you please encorporate that?

…named multi-file uploads in Nested Forms became malformed after editing the child entry.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
gravity-forms/gw-gravity-forms-rename-uploaded-files.php (2)

16-16: Version bump noted — sync docs/changelog.
Please update any visible snippet headers/README/changelog to reflect 2.7 and reference the Nested Forms edit fix.


101-104: Good fallback on rename failure; add debug logging.
Appending the original URL prevents data loss. Consider lightweight logging to aid support triage.

- if ( ! $result ) {
-     $renamed_files[] = $_file;
-     continue;
- }
+ if ( ! $result ) {
+     $renamed_files[] = $_file;
+     if ( defined( 'WP_DEBUG' ) && WP_DEBUG ) {
+         error_log( sprintf(
+             'GW_Rename_Uploaded_Files: rename() failed for entry %d: %s -> %s',
+             $entry['id'],
+             $file,
+             $renamed_file
+         ) );
+     }
+     continue;
+ }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 19305b2 and 705aa40.

📒 Files selected for processing (1)
  • gravity-forms/gw-gravity-forms-rename-uploaded-files.php (2 hunks)

Copy link
Contributor

@saifsultanc saifsultanc left a comment

Choose a reason for hiding this comment

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

LGTM!

@SebastianWiz SebastianWiz merged commit 692c93e into master Sep 11, 2025
4 checks passed
@SebastianWiz SebastianWiz deleted the seb/fix/88344-nested-forms-edit-malformed branch September 11, 2025 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants