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

Fixed issue #18324: mandSoft POST parameter may be passed to bypass mandatory questions #2825

Merged
merged 7 commits into from
Apr 3, 2023

Conversation

Shnoulle
Copy link
Collaborator

@Shnoulle Shnoulle commented Jan 4, 2023

Dev: move mandSoft control to _validateQuestion only

@sonarcloud
Copy link

sonarcloud bot commented Jan 4, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Shnoulle
Copy link
Collaborator Author

Shnoulle commented Jan 12, 2023

@olleharstedt i can assign @gabrieljenik as reviewer ?
I think its a big issue here (but i don't use mandatory soft option)

[edit : bad name for Gabriel, sorry … )

Copy link
Collaborator

@gabrieljenik gabrieljenik left a comment

Choose a reason for hiding this comment

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

Code structure looks OK. Not sure I understand the solution (don'tneed to :) )
Haven't tested it,

@Shnoulle
Copy link
Collaborator Author

Shnoulle commented Jan 12, 2023

Not sure I understand the solution (don'tneed to :) )

:) since maybe you need to fix EM :

  • before : checking of App()->request->getPost('mandSoft') on a lot of function and if question are Y or S
  • now : checking App()->request->getPost('mandSoft') only in _validateQuestion function and only for mandatory == 'S'.

Fibnally : if a group have one Y and one S and mandSoft is set : Y questions is set invalid (and stay invalid) : group is invalid.
Update only Y question : need to set mandSoft transoarently to the user.

@Shnoulle Shnoulle added Code review done Version checked for code issue without testing and removed Needs code review labels Jan 17, 2023
@Shnoulle
Copy link
Collaborator Author

Shnoulle commented Apr 2, 2023

Added auto test

Dev: move to MandatorySoftTest (with only testMandatorySoftAndMandatory function)
@sonarcloud
Copy link

sonarcloud bot commented Apr 3, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 10 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@olleharstedt olleharstedt merged commit fe2f996 into master Apr 3, 2023
@adamzammit
Copy link
Collaborator

This breaks the Mandatory soft function when used more than once over multiple pages/groups.

See here for the bug report: https://bugs.limesurvey.org/view.php?id=18808

@c-schmitz c-schmitz deleted the bug/18324_mandSoftBrokeMandatory branch June 20, 2023 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code review done Version checked for code issue without testing Has auto test Needs testing
Projects
None yet
4 participants