feat: allow editing of submission by the user by tpokorra · Pull Request #1690 · nextcloud/forms

tpokorra · 2023-08-18T10:01:41Z

related to #1110

show checked radio buttons and check boxes with previous answer
update submission and answers, don't delete and create submission or answers
allow deletion of submission
rebase to main branch
resolve Lint and Unit Test Job Failures
add Unit tests for new functionality

Chartman123

I just added some comments on the Changelog and the translations :)

I didn't have a closer look for now.

But one thing that I already saw: you seem to delete an existing submission instead of updating it. I'm not quite sure if that's the way to go here.

tpokorra · 2023-08-18T21:23:44Z

Thank you @Chartman123
I will fix those issues.

The whole pull request is not fully functional yet.
I still need to get the vue components to display the previously submitted answers.

Chartman123 · 2023-08-18T21:30:27Z

@tpokorra yes no problem :) you can have a look at the PR implementing the local storage how the components get their values there. And for getting the values from the database and pass them to the front-end can be seen in the results view...

tpokorra · 2023-08-23T10:30:24Z

TODO list:

show checked radio buttons and check boxes with previous answer
update answer, don't delete and create answer
allow deletion of answer

Chartman123 · 2023-08-23T11:15:25Z

@tpokorra I've added your checklist to first post :) And btw thank you very much for your contribution to this project 👍🏻

I've also run the automated check workflows for this PR so that you can easily find problems with your commits. Please have a look at the failed checks. If you have questions about some of the checks feel free to ask 🙂

Chartman123 · 2023-08-29T13:13:40Z

@tpokorra please don't merge the main branch into your branch. Use git rebase to update your branch to the latest commits to main :)

You can also use git rebase to squash your commits into a single one so that the commit history on your main branch stays clean :)

You can try git rebase -i main to do this :) After that you'll have to use git push --force to push the rebased commits to your branch.

tpokorra · 2023-10-23T15:53:28Z

@Chartman123
I am trying to run the unit tests locally, so that I can fix and extend them.

I have a small question: why is there a reference to Test\TestCase in most test files?

I get this error:

tim03-nextcloud@h13:~/nextcloud/apps/forms$ composer test:unit
> phpunit -c tests/phpunit.xml
PHP Fatal error:  Uncaught Error: Class "Test\TestCase" not found in /home/pacs/tim03/users/nextcloud/nextcloud/apps/forms/tests/Unit/Activity/ActivityManagerTest.php:41
Stack trace:

If I change it from Test\TestCase to PHPUnit\Framework\TestCase it just complains about the next file.

Am I missing something to install?

I ran:

cd nextcloud/apps/forms
composer install
composer dumpautoload -o

phpunit in vendor/bin shows version 9.6.13

Chartman123 · 2023-10-23T16:01:08Z

@tpokorra didn't manage to run the unit tests locally...

Regarding the extends \Test\TestCase you can also find some information in the dev docs: https://docs.nextcloud.com/server/stable/developer_manual/basics/testing.html

tpokorra · 2023-10-23T17:02:37Z

@Chartman123
Thank you, your hint helped.
I needed to copy the folder tests from https://github.com/nextcloud/server/tree/master/tests to my installation of Nextcloud.
I can now fix the test issues.

codecov · 2023-10-23T17:55:33Z

Codecov Report

Attention: Patch coverage is 63.15789% with 49 lines in your changes missing coverage. Please review.

Project coverage is 44.12%. Comparing base (39de0d7) to head (38cb8d6).
Report is 1 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1690      +/-   ##
============================================
+ Coverage     43.43%   44.12%   +0.68%     
- Complexity      882      919      +37     
============================================
  Files            77       78       +1     
  Lines          3361     3490     +129     
============================================
+ Hits           1460     1540      +80     
- Misses         1901     1950      +49

SergioArbarviro · 2024-03-18T21:41:22Z

Hello @tpokorra

I thank you very much for your good work. I fully support this feature, which I believe is very important in many use cases. Do you need help, and if so, where?

…User Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

AIlkiv

Thank you for your work. A few comments from me.

AIlkiv · 2025-03-01T14:50:18Z

+
+		// get existing submission of this user
+		try {
+			$submission = $this->submissionMapper->findByFormAndUser($form->getId(), $this->currentUser->getUID());


It makes sense to move the ownership check to submission before validateSubmission

It is better to rewrite this check to $this->submissionMapper->findById($submissionId). This will cover situations with multiple submissions and is a faster method.

AIlkiv · 2025-03-01T15:06:43Z

+		$submission->setTimestamp(time());
+		$this->submissionMapper->update($submission);
+
+		if (empty($answers)) {


In what cases can this be? When deleting a submission?

AIlkiv · 2025-03-01T15:16:44Z

+		// get stored answers for this question
+		$storedAnswers = [];
+		if ($update) {
+			$storedAnswers = $this->answerMapper->findBySubmissionAndQuestion($submissionId, $question['id']);


To generate fewer database queries, before foreach where the storeAnswersForQuestion method is called, get all the answers in one call through $this->answerMapper->findBySubmission and pass here

AIlkiv · 2025-03-01T15:36:06Z

+		$storedAnswers = [];
+		if ($update) {
+			$storedAnswers = $this->answerMapper->findBySubmissionAndQuestion($submissionId, $question['id']);
+		}


I will try to propose a simpler solution for this method

$deletingStoredAnswers = [];
foreach ($storedAnswers as $storedAnswer) {
$deletingStoredAnswers[$storedAnswer->getText() ] = $storedAnswer
}

AIlkiv · 2025-03-01T15:41:13Z

+			$storedAnswers = $this->answerMapper->findBySubmissionAndQuestion($submissionId, $question['id']);
+		}
+
+		$newAnswerTexts = [];


AIlkiv · 2025-03-01T15:45:15Z

 				$answerText = $name;
+
+				$answerEntity->setText($answerText);
+				$this->answerMapper->insert($answerEntity);


AIlkiv · 2025-03-01T15:47:19Z

+					$answerEntity->setQuestionId($question['id']);
+					$answerEntity->setText($answerText);
+					$this->answerMapper->insert($answerEntity);
+				}


AIlkiv · 2025-03-01T15:47:26Z

 			}

-			$answerEntity->setText($answerText);
-			$this->answerMapper->insert($answerEntity);


if (array_key_exists($answerText, $deletingStoredAnswers)) {
unset($deletingStoredAnswers[$answerText])
continue; // answer already been stored
}

$answerEntity->setText($answerText);
$this->answerMapper->insert($answerEntity);

AIlkiv · 2025-03-01T16:31:59Z

+					$this->answerMapper->delete($storedAnswer);
+				}
+			}
+		}


if (!empty( $deletingStoredAnswers) {
foreach ( $deletingStoredAnswers as $storedAnswer) {
$this->answerMapper->delete($storedAnswer);
}
}

AIlkiv · 2025-03-01T16:43:20Z

 		$result['questions'] = $this->getQuestions($form->getId());
+
+		// add previous submission if there is one by this user for this form
+		if ($this->currentUser->getUID() && $form->getAllowEdit()) {


if this only works for cases where only one submission is allowed. Add !$form->getSubmitMultiple() here

Chartman123 · 2025-04-14T08:33:51Z

@tpokorra I'll try to get this branch back from the forked repo into a feature branch in our upstream repo if it's fine for you :) Then we can start working on your great contributions to get it in shape and being able to release it soon :)

Chartman123 · 2025-04-21T18:56:19Z

Closed in favor of #2715

tpokorra marked this pull request as draft August 18, 2023 10:01

tpokorra force-pushed the TP-202308-allowEdit branch from b8974c8 to ebe6255 Compare August 18, 2023 10:06

Chartman123 reviewed Aug 18, 2023

View reviewed changes

Comment thread CHANGELOG.md

Comment thread l10n/de.js

Comment thread l10n/de.json

Comment thread l10n/de_DE.js

Comment thread l10n/de_DE.json

tpokorra force-pushed the TP-202308-allowEdit branch from f5cc31d to 09db0ef Compare August 28, 2023 18:12

tpokorra force-pushed the TP-202308-allowEdit branch from 76b1260 to 71f1622 Compare August 30, 2023 09:56

tpokorra force-pushed the TP-202308-allowEdit branch from 3f1f5db to b163c92 Compare October 23, 2023 14:26

Chartman123 added enhancement New feature or request php PHP related ticket javascript Javascript related ticket 2. developing Work in progress feature: 📝 submitting responses labels Oct 23, 2023

tpokorra force-pushed the TP-202308-allowEdit branch 4 times, most recently from 48e8988 to 6917908 Compare October 25, 2023 19:50

Chartman123 mentioned this pull request Nov 1, 2023

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

Merged

1 task

tpokorra mentioned this pull request Jan 5, 2024

refactoring: move code to separate function storeAnswersForQuestion #1866

Merged

tpokorra force-pushed the TP-202308-allowEdit branch 2 times, most recently from 9d148cd to d60b3c4 Compare January 5, 2024 16:37

SergioArbarviro mentioned this pull request Mar 18, 2024

Sharing option: delete own responses #2014

Closed

tpokorra added 21 commits February 25, 2025 18:58

fail silently for MultipleObjectsReturnedException from findByFormAnd…

68296b2

…User Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

updateSubmission: first check if editing is allowed and by this user

88ef56a

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

add integration test testUpdateSubmission

50c64bc

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

fix missing MultipleObjectsReturnedException for FormsService

e504d77

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

testUpdateSubmission: POST files instead of PUT

155ab5d

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

fixes for Psalm: add allowEdit

09cdbb6

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

fix ApiRoute for updateSubmission

bf941df

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

fix RespectAdminSettingsTest

b821904

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

fix openapi errors

821c9e4

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

another fix for openapi

94d2c9a

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

extend FormsForm for psalm

73be15f

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

update openapi.json

f301377

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

another fix for FormsForm

a347549

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

another fix for FormsForm

53e1d77

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

another attempt to fix openapi issues

cea5b82

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

simplify check in updateSubmission

712dd45

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

testUpdateSubmission: set AllowEdit for form

053d913

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

testUpdateSubmission: use different http client for user1

9a20f25

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

fix password for user1

5d1e27b

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

fix testUpdateSubmission

264b656

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

fix testUpdateSubmission

31af1b3

Signed-off-by: Timotheus Pokorra <timotheus.pokorra@solidcharity.com>

tpokorra force-pushed the TP-202308-allowEdit branch from 38cb8d6 to 31af1b3 Compare February 25, 2025 17:59

AIlkiv requested changes Mar 1, 2025

View reviewed changes

Chartman123 modified the milestones: 5.1, 5.2 Apr 5, 2025

Chartman123 mentioned this pull request Apr 21, 2025

feat: allow editing of submission by the user #2715

Merged

Chartman123 closed this Apr 21, 2025

Chartman123 removed this from the 5.2 milestone Jun 2, 2025

Conversation

tpokorra commented Aug 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Chartman123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tpokorra commented Aug 18, 2023

Uh oh!

Chartman123 commented Aug 18, 2023

Uh oh!

tpokorra commented Aug 23, 2023

Uh oh!

Chartman123 commented Aug 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Chartman123 commented Aug 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tpokorra commented Oct 23, 2023

Uh oh!

Chartman123 commented Oct 23, 2023

Uh oh!

tpokorra commented Oct 23, 2023

Uh oh!

codecov Bot commented Oct 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

SergioArbarviro commented Mar 18, 2024

Uh oh!

AIlkiv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Chartman123 commented Apr 14, 2025

Uh oh!

Chartman123 commented Apr 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tpokorra commented Aug 18, 2023 •

edited

Loading

Chartman123 commented Aug 23, 2023 •

edited

Loading

Chartman123 commented Aug 29, 2023 •

edited

Loading

codecov Bot commented Oct 23, 2023 •

edited

Loading