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

The fields are links to the files in a form result report #12474

Conversation

volha-pivavarchyk
Copy link
Contributor

Q A
Bug fix? (use the a.b branch) [✗]
New feature/enhancement? (use the a.x branch) [✓]
Deprecations? [✗]
BC breaks? (use the c.x branch) [✗]
Automated tests included? [ ]
Related user documentation PR URL mautic/mautic-documentation#...
Related developer documentation PR URL mautic/developer-documentation#...
Issue(s) addressed Fixes #...

Description:

Form fields with the file type added to the report cannot now be loaded. This PR fixes this.

Steps to test this PR:

  1. Open this PR on Gitpod or pull down for testing locally (see docs on testing PRs here)
  2. Check that the Use the form result as report data sources setting is enabled.
  3. Create a new form.
  4. Add a field with the file type and save the form.
  5. Fill the field and submit the form.
  6. Create a new report with the data source of the created result form.
  7. Check whether the file can be downloaded. Previously this was not possible.

@codecov
Copy link

codecov bot commented Jun 12, 2023

Codecov Report

Merging #12474 (3cb4c69) into 5.x (93c4a09) will increase coverage by 0.00%.
The diff coverage is 64.70%.

Impacted file tree graph

@@            Coverage Diff            @@
##                5.x   #12474   +/-   ##
=========================================
  Coverage     57.27%   57.27%           
- Complexity    33544    33549    +5     
=========================================
  Files          2158     2158           
  Lines        101329   101345   +16     
=========================================
+ Hits          58035    58050   +15     
- Misses        43294    43295    +1     
Impacted Files Coverage Δ
...dles/FormBundle/EventListener/ReportSubscriber.php 91.89% <0.00%> (-2.56%) ⬇️
...bundles/FormBundle/Controller/ResultController.php 39.75% <84.61%> (+3.94%) ⬆️

... and 3 files with indirect coverage changes

@sonarcloud
Copy link

sonarcloud bot commented Jun 13, 2023

Copy link
Sponsor Member

@escopecz escopecz left a comment

Choose a reason for hiding this comment

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

The code looks great 👍

I have not tested the enhancement

@escopecz escopecz added enhancement Any improvement to an existing feature or functionality pending-test-confirmation PR's that require one test before they can be merged reports Anything related to reports forms Anything related to forms labels Jul 20, 2023
Copy link

@PatrickJenkner PatrickJenkner left a comment

Choose a reason for hiding this comment

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

Works as described for a user.

@escopecz escopecz added this to the 5.0-beta milestone Jul 20, 2023
@escopecz escopecz merged commit 687c1ab into mautic:5.x Jul 20, 2023
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Any improvement to an existing feature or functionality forms Anything related to forms pending-test-confirmation PR's that require one test before they can be merged reports Anything related to reports
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

4 participants