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

[CORE-368] Validate entitytype rename #3212

Merged
merged 5 commits into from
Mar 7, 2025
Merged

Conversation

calypsomatic
Copy link
Contributor

@calypsomatic calypsomatic commented Mar 5, 2025

Ticket: https://broadworkbench.atlassian.net/browse/CORE-368

Adds in name validation when renaming an entity. DataBiosphere/terra-ui#5272 also does this from the front end.


PR checklist

  • Include the JIRA issue number in the PR description and title
  • Make sure Swagger is updated if API changes
    • ...and Orchestration's Swagger too!
  • If you changed anything in model/, then you should publish a new official rawls-model and perform the corresponding dependency updates as specified in the README:
    • in the automation subdirectory
    • in workbench-libs
    • in firecloud-orchestration
  • Get two thumbsworth of PR review
  • Verify all tests go green, including CI tests
  • Squash commits and merge to develop (branches are automatically deleted after merging)
  • Inform other teams of any substantial changes via Slack and/or email

@calypsomatic calypsomatic changed the title validate entitytype rename [CORE-368] Validate entitytype rename Mar 5, 2025
@calypsomatic calypsomatic marked this pull request as ready for review March 5, 2025 20:16
@calypsomatic calypsomatic requested a review from a team as a code owner March 5, 2025 20:16
@calypsomatic calypsomatic requested review from davidangb and kevinmarete and removed request for a team March 5, 2025 20:16
@@ -243,6 +245,7 @@ class EntityService(protected val ctx: RawlsRequestContext,
def renameEntityType(workspaceName: WorkspaceName, oldName: String, renameInfo: EntityTypeRename): Future[Int] = {
import org.broadinstitute.dsde.rawls.dataaccess.slick.{DataAccess, ReadAction}

validateEntityName(renameInfo.newName)
Copy link
Contributor

Choose a reason for hiding this comment

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

validateEntityName checks syntax for an entity name, not an entity type. See

private def validateEntity(entity: Entity): Unit = {
if (entity.entityType.equalsIgnoreCase(Attributable.workspaceEntityType)) {
throw new RawlsFatalExceptionWithErrorReport(
errorReport = ErrorReport(
message = s"Entity type ${Attributable.workspaceEntityType} is reserved and cannot be overwritten",
statusCode = StatusCodes.BadRequest
)
)
}
validateUserDefinedString(entity.entityType)
validateEntityName(entity.name)
entity.attributes.keys.foreach { attrName =>
validateUserDefinedString(attrName.name)
validateAttributeName(attrName, entity.entityType)
}
}
for the logic around validating entity types. You might wanna break out the two validations inside EntityComponent.validateEntity that look at entityType into a new method, then call that new method from here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah dang I looked very carefully at the different methods and still picked the wrong one 😵

@calypsomatic calypsomatic merged commit 6460d2e into develop Mar 7, 2025
29 checks passed
@calypsomatic calypsomatic deleted the core-368-rename branch March 7, 2025 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants