Skip to content

Implement max length for input fields on the feedback page#1557

Merged
osma merged 1 commit intoNatLibFi:masterfrom
ArchiXL:feedback-form-input-length-fix
Dec 5, 2023
Merged

Implement max length for input fields on the feedback page#1557
osma merged 1 commit intoNatLibFi:masterfrom
ArchiXL:feedback-form-input-length-fix

Conversation

@ghost
Copy link

@ghost ghost commented Nov 22, 2023 •
edited by ghost
Loading

Reasons for creating this PR

This PR adds a maximum input length to the "name", "email", and "subject" input fields on the feedback page, both to the frontend and backend. A maximum length was not previously implemented, and allows users to input very large strings either in the form, or by hijacking the POST-request.

Link to relevant issue(s), if any

  • none

Description of the changes in this PR

This PR imposes a limit of 255 characters to the "name", "email", and "subject" input fields on the feedback page; both in the frontend (feedback.twig, by using attribute "maxlength") and the backend (WebController, by using "substr").

Known problems or uncertainties in this PR

  • The limit of 255 characters is hard coded.

Checklist

  • phpUnit tests pass locally with my changes
  • I have added tests that show that the new code works, or tests are not relevant for this PR (e.g. only HTML/CSS changes)
  • The PR doesn't reduce accessibility of the front-end code (e.g. tab focus, scaling to different resolutions, use of .sr-only class, color contrast)
  • The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 22, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@codecov
Copy link

codecov bot commented Nov 22, 2023

Codecov Report

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

Comparison is base (88c2c78) 70.20% compared to head (aa8f16d) 70.20%.

Files Patch % Lines
controller/WebController.php 0.00% 3 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff            @@
##             master    #1557   +/-   ##
=========================================
  Coverage     70.20%   70.20%           
  Complexity     1664     1664           
=========================================
  Files            32       32           
  Lines          4293     4293           
=========================================
  Hits           3014     3014           
  Misses         1279     1279

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@osma osma added enhancement Skosmos 2.X Relevant for Skosmos 2 labels Dec 5, 2023
@osma osma added this to the 2.17 milestone Dec 5, 2023
@osma osma self-assigned this Dec 5, 2023
@osma
Copy link
Member

osma commented Dec 5, 2023

Thanks for the PR @rvdwxl !

Hardcoding the limit to 255 characters is a little ugly, but I think this is still an improvement, so this can be merged to Skosmos 2.x and included in the upcoming 2.17 release (probably the last one in the Skosmos 2.x) series.

For Skosmos 3 I think we need a more detailed specification on how to handle this. I will open a separate issue about that shortly.

@osma osma merged commit 3b9489a into NatLibFi:master Dec 5, 2023
@osma osma mentioned this pull request Dec 5, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@osma osma

Labels

enhancement Skosmos 2.X Relevant for Skosmos 2

Projects

None yet

Milestone

2.17

Development

Successfully merging this pull request may close these issues.

1 participant

@osma