Truncate description with gitlab project name #57
Conversation
…at are too long" This reverts commit 859bc53.
|
Hooray! All contributors have signed the CLA. |
shrestabangaru
changed the title
src/services/builds.ts
Outdated
| @@ -47,8 +48,12 @@ export const webhookPipelineEventToCompassBuildEvent = ( | |||
| build: { | |||
| externalEventSourceId: pipeline.project.id.toString(), | |||
| updateSequenceNumber: lastUpdated.getTime(), | |||
| displayName: `${pipeline.project.name} pipeline ${pipeline.object_attributes.id}`, | |||
| description: `Pipeline run ${pipeline.object_attributes.id} for project ${pipeline.project.name}`, | |||
| displayName: truncateProjectNameString(``, pipeline.project.name, ` pipeline ${pipeline.object_attributes.id}`), | |||
There was a problem hiding this comment.
Out of curiosity, is there a reason why you went with to define an empty string? I think the convention is probably to use "" and use whenever string interpolation is needed
There was a problem hiding this comment.
null causes issues because I am checking the string length for calculations, but where possible, `` has been changed to "".
src/services/deployment.ts
Outdated
| const projectNameLen = DESCRIPTION_TRUNCATION_LENGTH - beforeString.length - afterString.length; | ||
| truncatedProjectName = projectName.slice(0, projectNameLen); | ||
| } | ||
| return `${beforeString}${truncatedProjectName}${afterString}`; |
There was a problem hiding this comment.
Do we also need to truncate afterString at some point?
There was a problem hiding this comment.
Here is the list of before strings:
Pipeline run ${pipeline.id} for projectPipeline run ${pipeline.object_attributes.id} for project
And a list of after strings:pipelinedeploymentdeployment ${deployment.id}
where most of the id fields are <10 characters. Because the character limits for fields within a component are typically <100 or <255, we would not run into a scenario where truncating project name would not be enough.
That being said, maybe some alternate logic could be implemented for this to truncate all three strings in a way that makes the final string more intuitive? please let me know if this is what you had in mind!
There was a problem hiding this comment.
Yep I think I had thought we would return ${beforeString}${truncatedProjectName}${afterString}.slice(0, DESCRIPTION_TRUNCATION_LENGTH) to make sure that even if we start passing in some new after strings, we don't go past our length limit
src/services/deployment.ts
Outdated
| const projectNameLen = DESCRIPTION_TRUNCATION_LENGTH - beforeString.length - afterString.length; | ||
| truncatedProjectName = projectName.slice(0, projectNameLen); | ||
| } | ||
| return `${beforeString}${truncatedProjectName}${afterString}`; |
There was a problem hiding this comment.
Could we move this function to utils/event-mapping.ts file since it is shared between builds/deployment? Then we can also create a event-mapping.test.ts file to add some unit tests for this function.
There was a problem hiding this comment.
sounds good! Here are the test cases I'm thinking of:
- beforeString + projectName + afterString is short enough but >100 characters and doesn't need to be truncated
- ^ is too long and needs to be truncated to 255 characters
- ^ is exactly 255 characters and should not be truncated
- ^ is less than 100 characters and should not be truncated
src/services/deployment.ts
Outdated
| const projectNameLen = DESCRIPTION_TRUNCATION_LENGTH - beforeString.length - afterString.length; | ||
| truncatedProjectName = projectName.slice(0, projectNameLen); | ||
| } | ||
| return `${beforeString}${truncatedProjectName}${afterString}`; |
There was a problem hiding this comment.
Yep I think I had thought we would return ${beforeString}${truncatedProjectName}${afterString}.slice(0, DESCRIPTION_TRUNCATION_LENGTH) to make sure that even if we start passing in some new after strings, we don't go past our length limit
Description
This PR is to address the issue that when a project name in GitLab is too long, it causes errors/malfunctions in the data connection established between a Compass component and a GitLab group. GitLab project name limit is 255 characters, while a Compass component (and its various internal fields) have a limit of 100 characters. Thus, where necessary, this PR makes the changes to truncate description and display name fields to ensure that event data continues to be passed properly between GitLab and Compass despite different naming requirements.
Here is a screenshot showing successful builds appearing in Compass despite a long project name in GitLab:
Checklist
Please ensure that each of these items has been addressed: