Repository navigation
Conditional Logic - #3123
Conditional Logic#3123mickenordin wants to merge 30 commits into
Conversation
c7ff7a9 to
7887848
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
2d5f073 to
50a1acd
Compare
Updated with more tests so threshold is well exceeded |
50a1acd to
2250d5f
Compare
2250d5f to
3aea864
Compare
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
5c7d036 to
d02bdbc
Compare
d02bdbc to
78643a6
Compare
|
Thank you! Will test further. It seems a bit sluggish sometimes it throws an This is the missing feature i need to convince my non-profit to stop using google forms 🙏 |
|
Hi @mickenordin , as this was stale for 3 Months i started fixing some bugs by myself. I cant push on this branch, should i just post my .patch diffs here? Problems noticed/fixed:
Further ideas for this (should be new PR):
|
|
Yes
Yes, please do. I don't know of a better way since I don't have access to any access settings in the Nextcloud org, even though I was given the ability to branch in their repos myself. I'll make sure to credit you on the diffs of course. |
|
Hey @mickenordin |
78643a6 to
532bee6
Compare
I did, please make sure to sign-off your commits, and perhaps squash them to fewer logical units. You can take a look at the failing tests and see how to fix them. |
Signed-off-by: TimedIn <git@timedin.net>
Signed-off-by: TimedIn <git@timedin.net>
- update test to mock getActiveBranches (plural) - type getActiveBranches/findActiveBranches as non-nullable array - add triggerType and branches to extra settings type - simplify branch guard in question cloning Signed-off-by: Micke Nordin <kano@sunet.se>
Signed-off-by: Micke Nordin <kano@sunet.se>
Signed-off-by: Micke Nordin <kano@sunet.se>
Signed-off-by: Micke Nordin <kano@sunet.se>
Signed-off-by: Micke Nordin <kano@sunet.se>
Signed-off-by: Micke Nordin <kano@sunet.se>
|
I'll do another rebase and will tackle the code review findings for starters. Scope discussion could than be next 👍 |
updateQuestion() accepted parentQuestionId and branchId, so a question could be moved under a conditional question of another form. Both are now fixed on creation, like options and accept, which are not columns of a question at all. Changing a conditional question into another type, or a question into a conditional one, would leave subquestions without a parent or nest conditional questions, so these type changes are rejected as well. newQuestion() now also checks that the branch exists on the parent. Subquestions are loaded together with the form's questions in one query, with their options in a second one, instead of two queries per conditional question, and only subquestions of the parent's own form are attached to its branches. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
…sistent - Removing a branch deletes its subquestions instead of leaving them behind as hidden export columns, and the stored branches no longer keep a copy of their subquestions. - Deleting or copying a subquestion renumbers or orders within its branch instead of shifting the top-level questions. - Copying a conditional question, alone or with its form, copies the subquestion rows and points the branch conditions to the options of the copy. The copy used to have empty branches whose conditions referred to the options of the original. - Exports list the subquestions of a conditional question right after it, in branch order, instead of interleaving them by their order. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
…stions The trigger answer of a conditional question skipped every check a regular question gets, and subquestion answers were stored by a copy of the store logic that missed ranking, grid and "other" answers. - The trigger is validated and stored as a question of the trigger type, so unknown options are rejected, an empty trigger does not satisfy a required conditional question and file triggers are kept. - Subquestion answers go through the regular store logic. - Dropdowns send option ids as numbers, which never matched the string comparison, so their branches were never active on the server and the subquestion answers were dropped. - A checkbox branch is active if any of its option combinations is selected, like in the browser. - An unanswered trigger activates no branch. - Regex conditions use the delimited format of the short answer validation, which the server needs, and are checked when saved. Saved conditions must use a condition type of the trigger type. - The unused list of all condition types is removed. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
…tions
Subquestions used the index of the next top-level question, so element
ids were duplicated and inputs got the label of another question.
- Subquestions are numbered after their conditional question ("1.2"),
across all of its branches, and their element ids are derived from
that, the trigger's own elements get ids of their own.
- The trigger is rendered inside the conditional question's list item
instead of as a second list item, and the subquestions shown while
answering are a nested list.
- A polite status message tells screen reader users when follow-up
questions appear or disappear.
- In the editor each branch is a group named after its condition, its
buttons name the branch, the heading follows the question title's
level and names the question, and the condition controls have
labels.
- The unused isTrigger prop is removed.
Assisted-by: Claude Code:claude-opus-5-5
Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
… the server - Answers of subquestions in branches that are no longer shown are not sent anymore, the server rejected them as answers for unknown questions. They are kept in the browser in case the trigger is changed back. - Changing the trigger type opens the type selection again instead of saving an empty trigger type, which the server rejected. Settings of the old trigger type are dropped, the conditions of the branches are reset and their subquestions are kept. - Regular expressions are entered as /pattern/modifiers, like for the short answer validation, are checked while typing and matched the same way in the browser as on the server. - Date and time conditions are stored in the answer format instead of as a timestamp string that the server could not read, and compared without the time of day, so a range includes its bounds. - A file trigger without an upload activates the "no file uploaded" branches in the browser as well. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
… its options - The results summary shows the trigger of a conditional question as a question of its trigger type, followed by the subquestions of all branches, which were missing there. - The menu of the trigger only has the options of the trigger type. Required, technical name and delete belong to the conditional question and changed it from a second place, with the trigger always showing as not required. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Builds a form with a dropdown conditional question through the API and answers it in the browser: only the subquestions of the chosen answer are shown and stored, subquestions are numbered after their question without taking the label or ids of the questions around them. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
b1350c4 to
b193e0c
Compare
|
I rebased this branch on Split into a stackFollowing @Chartman123's suggestion (#3123 (comment)), the same code is now split into six stacked PRs, each based on the one before - I didn't rework this PR on purpose to keep the/an original copy :
#3833 contains all commits of this PR, unchanged, plus the review fixes. Its last commit takes out the trigger types that #3834–#3838 add back. Together, the six PRs have the scope of this PR. The base PR #3833 also:
#3834 also fixes the label of checkbox branches. @timedin-de's list (#3123 (comment))Checked on a dev instance:
#3123 (comment)
Regression from #3123 (comment)Confirmed fixed. File answers, in subquestions too, are checked against the upload token of the same form. An upload with a wrong token is rejected with 400. Found on the way, not caused by this PR
|
|
To give a bit of an explanation about the PR move and split. Given the discussion around what conditions/types to support. I've split it into the baseline (that seems generally agreed on in terms of supported types - like Google offers) and PRs per extra set, so we can discuss them after have the foundation in and see if there are use cases that justify adding them in addition. |
A text trigger activates a branch when the answer equals (short text only) or contains a text, or matches a regular expression. Regular expressions use the same /pattern/modifiers format as validationRegex, are checked when saving, and an invalid one is not saved by the editor. Matching is limited in subject length and backtracking. The code comes from the original pull request #3123 by Micke Nordin and TimedIn, with the review fixes applied. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
A text trigger activates a branch when the answer equals (short text only) or contains a text, or matches a regular expression. Regular expressions use the same /pattern/modifiers format as validationRegex, are checked when saving, and an invalid one is not saved by the editor. Matching is limited in subject length and backtracking. The code comes from the original pull request #3123 by Micke Nordin and TimedIn, with the review fixes applied. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
A text trigger activates a branch when the answer equals (short text only) or contains a text, or matches a regular expression. Regular expressions use the same /pattern/modifiers format as validationRegex, are checked when saving, and an invalid one is not saved by the editor. Matching is limited in subject length and backtracking. The code comes from the original pull request #3123 by Micke Nordin and TimedIn, with the review fixes applied. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
This is an implementation of Conditional Logic. It is not well tested, and I would appreciate review and testing by others to see if it works as intended. I have created a few forms and clicked around to the best of my abilities, but I have implemented the fix for the benefit of others that need it and have no real use case myself to test with.
Fixes: #358
🖼️ Screenshoots