Added possibility to link spreadsheet for automatic submission export in multiple formats #1758

Koc · 2023-10-22T22:14:06Z

Hello everyone!

I've added possibility to link form with spreadsheet in csv/ods/xlsx format and automatically export it once new submission added. Inspired from #1403 but with more formats. Also we edit already existent file, which give us possibility to add additional columns with notes/statuses/etc.

Here you can find few video/screenshots:

nextcloud-forms-export-03.mp4

Exported file

What is out of scope right now but can be added in upcoming PRs:

handling of responses deletion
handling of adding/removal of the questions

Todo:

fix old tests and add new

Chartman123 · 2023-10-23T06:14:05Z

@Koc thank you very much for your contribution! We'll have a look at it 🙂

@nextcloud/designers Could you please do a quick review of the screenshots? I'm not really convinced of the UI in the menu. Looks a bit crowded...

@hamza221 fine for you to close your PR and keep working on this one?

susnux

Thank you very much! Nice work!

Some first thought about your code, besides there are quite a lot of linter issues to fix.
I think we do not need to implement this as a breaking change API wise but can make this compatible with the current API by just extending.

Moreover I do not know if it makes much sense to store the file path in the database. We probably should just work with the file id.

appinfo/routes.php

lib/Controller/ApiController.php

lib/Migration/Version030200Date20231001200100.php

src/views/Results.vue

Koc · 2023-10-23T16:09:33Z

Mates, any idea how to fix that?

Error: lib/Service/SubmissionService.php:175:12: MissingDependency: OCP\Files\IRootFolder depends on class or interface oc\hooks\emitter that does not exist (see https://psalm.dev/157)

I haven't touched \OCP\Files\IRootFolder $storage dependency at all

Also I'm trying to run Psalm locally and got 27 errors which is completely unrelated to my changes and not present in pipeline 😕

Also pipeline not starting for my commits due to lack of approvals 🤷‍♂️

Koc · 2023-10-23T17:16:40Z

@susnux

Moreover I do not know if it makes much sense to store the file path in the database. We probably should just work with the file id.

Yes, it's doable but little bit tricky because we can choose file or folder. If it is a blocker for merge - just say and I will rework. Because now it sounds like you are not 100% sure :)

susnux · 2023-10-24T10:37:29Z

I haven't touched \OCP\Files\IRootFolder $storage dependency at all

Yes that error is unrelated, needs to be fixed on the server, so you can ignore it.
But the eslint issues still need to be fixed :)

Yes, it's doable but little bit tricky because we can choose file or folder.

Folders also have a file id. The reason why I would prefer to only use the ID rather then hard coding the path is:

If you rename on of the folders the path would be broken
If we (in future) support collaboration of forms the same folder will or can have a different path for different users. (depending if shared etc)

So it would make things more robust and safe for the future if we already stick with the ID.

nimishavijay · 2023-10-24T17:08:57Z

Super nice! :)
I have some questions:

Is the spreadsheet automatically updated when new responses are added?
What does "re-export spreadsheet" do? There are several actions on the screen that are closely related (create spreadsheet, save, download, re-export) so it could get a bit confusing to someone new

I do agree that the action menu looks a bit crowded, we can easily fix that :)

Instead of disabling, we could hide the "Re-export" and "Unlink" actions if a spreadsheet has not been created yet
For "Save" and "Download" we could use a second level action menu, similar to the send later action in mail

Recording.2023-10-24.190452.mp4

What do you think?

susnux · 2023-10-25T16:38:55Z

Error: lib/Service/SubmissionService.php:175:12: MissingDependency: OCP\Files\IRootFolder depends on class or interface oc\hooks\emitter that does not exist (see https://psalm.dev/157)

Can be silenced like this:
https://github.com/nextcloud/spreed/blob/55e5fa00ffcdafe3eb3a94bc03b25e6f2ee9982d/psalm.xml#L91

Koc · 2023-10-26T15:45:19Z

@nimishavijay yes, we are automatically update spreadsheet once new response added. But sometimes we can get into trouble with a race conditions. Imagine that Employee edits file with submissions in a browser and overwrites changes that exported after new response added. In this case we should have mechanism to force re-export responses to file after Employee finishes his work.

Koc · 2023-10-31T15:53:59Z

@nimishavijay I've added nested menu, you can find a video record for it in the PR description. Also disabling of non-active buttons has been replaced with hidding.

@susnux I've reworked PR, now we are storing fileId, fileFormat only, without filePath.

Also I've fixed all pipelines (I hope 🤞 ). Can we enable full pipeline for an every push commit for this PR?

Chartman123 · 2023-10-31T17:41:15Z

@Koc The workflows will run automatically once you have contributed to this repository. Until then we have to manually start it :)

codecov · 2023-11-01T09:18:56Z

Codecov Report

Attention: 65 lines in your changes are missing coverage. Please review.

Comparison is base (a6a4264) 44.61% compared to head (d6f5cb6) 44.79%.
Report is 7 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1758      +/-   ##
============================================
+ Coverage     44.61%   44.79%   +0.17%     
- Complexity      662      686      +24     
============================================
  Files            58       59       +1     
  Lines          2600     2697      +97     
============================================
+ Hits           1160     1208      +48     
- Misses         1440     1489      +49

Koc · 2023-11-09T23:38:18Z

wow, we have completely green pipeline for the very first time! 🎉

Chartman123

I had a look at the code, but not everything in-depth. Added some comments :) I will run the code in the next few days and see how it works.

lib/Service/SubmissionService.php

appinfo/routes.php

lib/Controller/ApiController.php

lib/Service/SubmissionService.php

tests/Integration/Api/ApiV2Test.php

Chartman123 · 2024-01-18T22:27:33Z

My bad... I forgot to escape the ' in one suggestion in ApiController.php

lib/Controller/ApiController.php

susnux

This works and I think we are close to merge!

I just have some nitpicks and some issues like wrong copyright, wrong version in migration and one more specific issue:
I still would like to see the API using fileids if possible. Or do you have some reason for using paths there?

docs/API.md

susnux · 2024-01-21T16:58:40Z

docs/API.md

@@ -132,6 +139,9 @@ Returns the full-depth object of the requested form (without submissions).
    "showToAllUsers": false
  },
  "expires": 0,
+  "fileFormat": "csv",
+  "fileId": 157,
+  "filePath": "foo/bar",


Is this the real path? If yes we should change it, as everywhere else in nextcloud we use absolute paths to the root of a user (which is for DAV /files/USERID thus the path would be /foo/bar).

susnux · 2024-01-21T16:58:54Z

docs/API.md

+"data": {
+  "fileFormat": "csv",
+  "fileId": 157,
+  "filePath": "foo/bar",


same as above

susnux · 2024-01-21T17:00:12Z

lib/Controller/ApiController.php

+	 * @throws OCSForbiddenException
+	 */
+	public function linkFile(string $hash, string $path, string $fileFormat): DataResponse {
+		$this->logger->debug('Linking form {hash} to file at /{path} in format {fileFormat}', [


see above the path should be absolute so you can get ride of the manual / before

lib/Migration/Version030200Date20231001200100.php

susnux · 2024-01-21T17:08:21Z

lib/Service/SubmissionService.php


-		// Get Data
-		$csvData = $this->getSubmissionsCsv($hash);
+		$this->logger->debug('Export submissions for form: {hash} to Cloud at: /{path} in format {fileFormat}', [


same as above probably should be absolute path so the extra / can be removed

it's written the same like it now in current main branch.

lib/Service/SubmissionService.php

susnux · 2024-01-21T17:18:31Z

lib/Service/SubmissionService.php

+
+		/** @var \OCP\Files\Folder|File $node */
+		if ($ownerId) {
+			$node = $this->storage->getUserFolder($ownerId)->get($path);


As said above, I do not think relative paths make much sense, I would either use an absolute one and remove the leading / here.
Or as initally said just use the file id, you can simply get the file then with $this->storage->getById(FILEID) (this returns an array if I am right, just take the first node then).

no, we can't use fileId here because writeFileToCloud method used in "save copy to files" to arbitrary path without linking

Also about relative vs absolute path: current codebase also uses relative path. I'm using literally same method.

Right this PR is big enough lets do optional refactoring in the future and keep the current way :)

appinfo/routes.php

susnux

Nice work! Just fix the rest of the routes (API version) then this is good to go 🎉

…xport in multiple formats Signed-off-by: Konstantin Myakshin <[email protected]>

Signed-off-by: Ferdinand Thiessen <[email protected]>

susnux · 2024-01-23T22:50:52Z

@Koc Thank you very much for this fantastic work! 🚀
I really like the feature and I am very happy you kept on working on this, I hope we can release it with 4.1.0 soon!

Koc · 2024-01-23T23:46:52Z

very happy to hear that, thanx! It was hard 😃

Chartman123 · 2024-01-24T07:34:33Z

Yes, thank you very much @Koc :) We'd be glad if you continue contributing here 🙂

jancborchardt · 2024-01-26T10:34:02Z

Awesome work @Koc! Do you already have a guest account on our instance? We could add you there so you can join the Forms community Talk channel. :)
(I just need an email address to add you with)

Koc · 2024-01-27T22:32:18Z

@jancborchardt no, I haven't. You can find my email address in copyrights 😃

Gamechiefx · 2024-02-07T04:17:46Z

Having this error when attempting to create a spreadsheet:

"no app in context","method":"POST","url":"/ocs/v2.php/apps/forms/api/v2.4/form/link/ods","message":"Exception thrown: OCP\Files\NotFoundException","userAgent":"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/121.0.0.0 Safari/537.36","version":"28.0.2.5","exception":{"Exception":"OCP\Files\NotFoundException","Message":"/admin/files/Documents/folder/TCC \nData Usage Assessment Form (responses).ods","Code":0,"Trace":[{"file":"/var/www/html/lib/private/Files/Node/Folder.php","line":135,"function":"get","class":"OC\Files\Node\Root","type":"->","args":["/TCC/files/Documents/folder/TCC \nData Usage Assessment Form (responses).ods"]},{"file":"/var/www/html/custom_apps/forms/lib/Service/SubmissionService.php","line":216,"function":"get","class":"OC\Files\Node\Folder","type":"->","args":["TCC \nData Usage Assessment Form (responses).ods"]},{"file":"/var/www/html/custom_apps/forms/lib/Controller/ApiController.php","line":1324,"function":"writeFileToCloud","class":"OCA\Forms\Service\SubmissionService","type":"->","args":[["OCA\Forms\Db\Form",1],"/Documents/folder","ods"]},{"file":"/var/www/html/lib/private/AppFramework/Http/Dispatcher.php","line":230,"function":"linkFile","class":"OCA\Forms\Controller\ApiController","type":"->","args":["LZyTtsgDfqtFWbNo","/Documents/folder","ods"]},{"file":"/var/www/html/lib/private/AppFramework/Http/Dispatcher.php","line":137,"function":"executeController","class":"OC\AppFramework\Http\Dispatcher","type":"->","args":[["OCA\Forms\Controller\ApiController"],"linkFile"]},{"file":"/var/www/html/lib/private/AppFramework/App.php","line":184,"function":"dispatch","class":"OC\AppFramework\Http\Dispatcher","type":"->","args":[["OCA\Forms\Controller\ApiController"],"linkFile"]},{"file":"/var/www/html/lib/private/Route/Router.php","line":315,"function":"main","class":"OC\AppFramework\App","type":"::","args":["OCA\Forms\Controller\ApiController","linkFile",["OC\AppFramework\DependencyInjection\DIContainer"],["v2.4","ods","ocs.forms.api.linkFile"]]},{"file":"/var/www/html/ocs/v1.php","line":65,"function":"match","class":"OC\Route\Router","type":"->","args":["/ocsapp/apps/forms/api/v2.4/form/link/ods"]},{"file":"/var/www/html/ocs/v2.php","line":23,"args":["/var/www/html/ocs/v1.php"],"function":"require_once"}],"File":"/var/www/html/lib/private/Files/Node/Root.php","Line":207,"CustomMessage":"Exception thrown: OCP\Files\NotFoundException"},"id":"65c302dad2cd8"}

Chat GPT Findings:

The error log you've provided outlines a NotFoundException occurring within a Nextcloud instance, specifically when trying to access or link to an .ods file located in a user's documents folder. Here's a detailed breakdown of what happened, the probable cause, and how to address it:

Error Summary

Exception Type: OCP\Files\NotFoundException
Affected Component: The Nextcloud Forms app (custom_apps/forms), particularly its SubmissionService and ApiController.
User Action: Attempting to link an .ods file ("TCC Data Usage Assessment Form (responses).ods") to a form submission.
System Response: The system was unable to find the specified .ods file in the path provided.

Probable Cause

The error was triggered during a POST request to link an .ods file to a form submission. The path to the file includes a line break (\n) which might be unintentional or misinterpreted by the system as part of the file path. File paths typically do not include line breaks, and their presence can cause the system to search for a file in a non-existent location.

Troubleshooting Actions

Attempted to remove and reinstall the app

Version

Latest Nextcloud 28

Chartman123 · 2024-02-07T07:53:47Z

@Gamechiefx please create a new issue for that and use the template for bug reports. Thank you!

matthijskooijman · 2024-02-18T13:59:42Z

You're other proposals will definitely not make it into this PR anymore. So please open separate feature request issues for them. We can then discuss them there.

About the technical name: the little error should be indeed for saving the values and this function is available since a few versions already. So as you said not related. Please also add a new issue for that.

Done, I've opened #1970, #1971, #1972 and #1973 for these issues.

JocelynDelalande · 2024-09-17T16:51:48Z

Hello, I think this feature is what I look for, but I don't understand how to use it. Is it documented somewhere ? Thanks by advance.

Chartman123 · 2024-09-17T21:19:35Z

@JocelynDelalande Just have a look at the video in the OP. :)

JocelynDelalande · 2024-09-18T08:41:00Z

@Chartman123 Thanks for your answer, and nevermind: I was just using a version of NC that was too old for forms 4.1. Can't wait to test the new Forms :)

Chartman123 added enhancement New feature or request 2. developing Work in progress design Related to the design feature: 📊 responses & statistics labels Oct 23, 2023

susnux reviewed Oct 23, 2023

View reviewed changes

Koc force-pushed the feature/link-forms-with-documents branch from 9cb08e7 to adac3e2 Compare October 23, 2023 17:00

Koc force-pushed the feature/link-forms-with-documents branch from 533353d to f0580ec Compare October 31, 2023 15:46

Koc force-pushed the feature/link-forms-with-documents branch from f0580ec to 065381d Compare October 31, 2023 22:34

This comment was marked as resolved.

Sign in to view

Koc force-pushed the feature/link-forms-with-documents branch from 065381d to 3b66bbc Compare November 1, 2023 08:54

This comment was marked as resolved.

Sign in to view

Koc force-pushed the feature/link-forms-with-documents branch from 3b66bbc to 035a318 Compare November 9, 2023 00:10

This comment was marked as resolved.

Sign in to view

This comment was marked as outdated.

Sign in to view

Chartman123 reviewed Nov 10, 2023

View reviewed changes

Chartman123 reviewed Jan 18, 2024

View reviewed changes

lib/Controller/ApiController.php Outdated Show resolved Hide resolved

susnux requested changes Jan 21, 2024

View reviewed changes

susnux reviewed Jan 21, 2024

View reviewed changes

appinfo/routes.php Outdated Show resolved Hide resolved

Koc force-pushed the feature/link-forms-with-documents branch 2 times, most recently from 0a72b8c to 7d9d4e5 Compare January 22, 2024 23:15

susnux reviewed Jan 23, 2024

View reviewed changes

appinfo/routes.php Outdated Show resolved Hide resolved

susnux approved these changes Jan 23, 2024

View reviewed changes

Koc and others added 2 commits January 23, 2024 23:45

enh: Added possibility to link spreadsheet for automatic submission e…

8d2b52b

…xport in multiple formats Signed-off-by: Konstantin Myakshin <[email protected]>

fix: Adjust routes api version

d6f5cb6

Signed-off-by: Ferdinand Thiessen <[email protected]>

susnux force-pushed the feature/link-forms-with-documents branch from 3815adb to d6f5cb6 Compare January 23, 2024 22:45

susnux merged commit 59d074e into nextcloud:main Jan 23, 2024
47 checks passed

Chartman123 linked an issue Jan 26, 2024 that may be closed by this pull request

Forms exports with line feeds (LF), that makes analysing the export complicated #1775

Closed

Koc deleted the feature/link-forms-with-documents branch September 18, 2024 15:33

tcitworld mentioned this pull request Oct 4, 2024

fix(export): remove new lines from form title in the exported filename #2343

Merged

Added possibility to link spreadsheet for automatic submission export in multiple formats #1758

Added possibility to link spreadsheet for automatic submission export in multiple formats #1758

Conversation

Koc commented Oct 22, 2023 • edited Loading

Chartman123 commented Oct 23, 2023

susnux left a comment

Choose a reason for hiding this comment

Koc commented Oct 23, 2023 • edited Loading

Koc commented Oct 23, 2023 • edited Loading

susnux commented Oct 24, 2023

nimishavijay commented Oct 24, 2023

susnux commented Oct 25, 2023

Koc commented Oct 26, 2023

Koc commented Oct 31, 2023

Chartman123 commented Oct 31, 2023

This comment was marked as resolved.

This comment was marked as resolved.

codecov bot commented Nov 1, 2023 • edited Loading

Codecov Report

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

Koc commented Nov 9, 2023

Chartman123 left a comment

Choose a reason for hiding this comment

Chartman123 commented Jan 18, 2024

susnux left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

susnux left a comment

Choose a reason for hiding this comment

susnux commented Jan 23, 2024

Koc commented Jan 23, 2024

Chartman123 commented Jan 24, 2024

jancborchardt commented Jan 26, 2024

Koc commented Jan 27, 2024

Gamechiefx commented Feb 7, 2024 • edited Loading

Troubleshooting Actions

Version

Chartman123 commented Feb 7, 2024

matthijskooijman commented Feb 18, 2024

JocelynDelalande commented Sep 17, 2024

Chartman123 commented Sep 17, 2024

JocelynDelalande commented Sep 18, 2024

Koc commented Oct 22, 2023 •

edited

Loading

Koc commented Oct 23, 2023 •

edited

Loading

Koc commented Oct 23, 2023 •

edited

Loading

codecov bot commented Nov 1, 2023 •

edited

Loading

Gamechiefx commented Feb 7, 2024 •

edited

Loading