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

allow editing of submission by the user #1690

Open
wants to merge 27 commits into
base: main
Choose a base branch
from

Conversation

tpokorra
Copy link
Contributor

@tpokorra tpokorra commented Aug 18, 2023

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

@tpokorra tpokorra marked this pull request as draft August 18, 2023 10:01
@tpokorra tpokorra force-pushed the TP-202308-allowEdit branch from b8974c8 to ebe6255 Compare August 18, 2023 10:06
Copy link
Collaborator

@Chartman123 Chartman123 left a comment

Choose a reason for hiding this comment

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

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.

CHANGELOG.md Outdated Show resolved Hide resolved
l10n/de.js Outdated Show resolved Hide resolved
l10n/de.json Outdated Show resolved Hide resolved
l10n/de_DE.js Outdated Show resolved Hide resolved
l10n/de_DE.json Outdated Show resolved Hide resolved
@tpokorra
Copy link
Contributor Author

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
Copy link
Collaborator

@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
Copy link
Contributor Author

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
Copy link
Collaborator

Chartman123 commented Aug 23, 2023

@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 🙂

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

Chartman123 commented Aug 29, 2023

@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 tpokorra force-pushed the TP-202308-allowEdit branch from 76b1260 to 71f1622 Compare August 30, 2023 09:56
@tpokorra tpokorra force-pushed the TP-202308-allowEdit branch from 3f1f5db to b163c92 Compare October 23, 2023 14:26
@Chartman123 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
Copy link
Contributor Author

@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
Copy link
Collaborator

@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
Copy link
Contributor Author

@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
Copy link

codecov bot commented Oct 23, 2023

Codecov Report

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

Project coverage is 43.37%. Comparing base (2ea7409) to head (095264b).
Report is 5 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1690      +/-   ##
============================================
- Coverage     43.51%   43.37%   -0.14%     
- Complexity      870      908      +38     
============================================
  Files            75       76       +1     
  Lines          3330     3421      +91     
============================================
+ Hits           1449     1484      +35     
- Misses         1881     1937      +56     

@SergioArbarviro
Copy link

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?

@tpokorra tpokorra force-pushed the TP-202308-allowEdit branch from 48f92a5 to ef36232 Compare January 9, 2025 18:52
Signed-off-by: Timotheus Pokorra <[email protected]>
Signed-off-by: Timotheus Pokorra <[email protected]>
Signed-off-by: Timotheus Pokorra <[email protected]>
Signed-off-by: Timotheus Pokorra <[email protected]>
Signed-off-by: Timotheus Pokorra <[email protected]>
Signed-off-by: Timotheus Pokorra <[email protected]>
…heckAndPrepareSubmission

Signed-off-by: Timotheus Pokorra <[email protected]>
Signed-off-by: Timotheus Pokorra <[email protected]>
Signed-off-by: Timotheus Pokorra <[email protected]>
Signed-off-by: Timotheus Pokorra <[email protected]>
Signed-off-by: Timotheus Pokorra <[email protected]>
@tpokorra tpokorra force-pushed the TP-202308-allowEdit branch from 767fd61 to 556b5f2 Compare January 9, 2025 19:03
*/
public function testUpdateSubmission() {

$uploadedFileResponse = $this->http->request('PUT',
Copy link
Collaborator

@Chartman123 Chartman123 Jan 11, 2025

Choose a reason for hiding this comment

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

This files need to be POSTed, there's no PUT route available for updating/changing them :)

Suggested change
$uploadedFileResponse = $this->http->request('PUT',
$uploadedFileResponse = $this->http->request('POST',

Copy link
Collaborator

Choose a reason for hiding this comment

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

As an alternative you could implement a function to update/change the files on updating the submission. Then the old files should be replaced by the new ones

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature or request feature: 📝 submitting responses javascript Javascript related ticket php PHP related ticket
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants