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

[API] Add support for project names with space #7463

Merged

Conversation

laemtl
Copy link
Contributor

@laemtl laemtl commented Jun 7, 2021

Remark: Project names are not designed to be unique in the DB. In case of duplicates, the Project factory fails here (pselectRow used on a query that returns more than one row). (reported in #7464)

@laemtl laemtl changed the title 2021 06 07 support project name with space [API] Add support for project name with space Jun 7, 2021
@laemtl laemtl force-pushed the 2021-06-07-support-project-name-with-space branch 3 times, most recently from e4df353 to 19d2c62 Compare June 7, 2021 15:09
@laemtl laemtl changed the title [API] Add support for project name with space [API] Add support for project names with space Jun 7, 2021
@laemtl laemtl requested a review from xlecours June 7, 2021 15:11
@laemtl laemtl added the API PR or issue introducing/requiring modifications to the API code label Jun 7, 2021
Comment on lines 104 to 106
$project_name = isset($pathparts[1]) ?
urldecode($pathparts[1]) :
'';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
$project_name = isset($pathparts[1]) ?
urldecode($pathparts[1]) :
'';
$project_name = urldecode($pathparts[1] ?? '');

Copy link
Contributor

@xlecours xlecours left a comment

Choose a reason for hiding this comment

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

I made a suggestion. Feel free to include it or not. The null coalesce operator is just a shorter version.
Other than that, all good.

@xlecours
Copy link
Contributor

xlecours commented Jun 7, 2021

And regarding the unicity of the project name, I think it is a good example of why the generic ID column makes a poor primary key in most situation.
Thanks for creating #7464

@laemtl laemtl force-pushed the 2021-06-07-support-project-name-with-space branch from 19d2c62 to 05ae90d Compare June 7, 2021 20:59
@laemtl
Copy link
Contributor Author

laemtl commented Jun 7, 2021

I made a suggestion. Feel free to include it or not. The null coalesce operator is just a shorter version.
Other than that, all good.

Good suggestion, thanks!

@driusan driusan merged commit 64a8ed6 into aces:main Jun 10, 2021
AlexandraLivadas pushed a commit to AlexandraLivadas/Loris that referenced this pull request Jun 29, 2021
Handle project names which need to be URL encoded in the API, such as projects with spaces.
@ridz1208 ridz1208 modified the milestone: 24.0.0 Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API PR or issue introducing/requiring modifications to the API code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants