Skip to content

fix: Feature can be updated through another release #268 #293

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.

Sign up for GitHub

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

Open
ElizavetaP wants to merge 1 commit into
base: toloka-agent/130-auto
Choose a base branch
from
Open

fix: Feature can be updated through another release #268 #293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/main/java/com/sivalabs/ft/features/api/controllers/ReleaseController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ ResponseEntity<Void> updateFeaturePlanning(
var username = SecurityUtils.getCurrentUsername();
var cmd = new UpdateFeaturePlanningCommand(
featureCode,
releaseCode,
payload.plannedCompletionDate(),
payload.planningStatus(),
payload.featureOwner(),
Expand Down Expand Up @@ -306,7 +307,7 @@ ResponseEntity<Void> removeFeatureFromRelease(
SecurityUtils.requireRole("ADMIN");
var username = SecurityUtils.getCurrentUsername();
var rationale = payload != null ? payload.rationale() : null;
var cmd = new RemoveFeatureFromReleaseCommand(featureCode, rationale, username);
var cmd = new RemoveFeatureFromReleaseCommand(featureCode, releaseCode, rationale, username);
featureService.removeFeatureFromRelease(cmd);
log.info("Feature {} removed from release {} by user {}", featureCode, releaseCode, username);
return ResponseEntity.ok().build();
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/com/sivalabs/ft/features/domain/Commands.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public record AssignFeatureToReleaseCommand(

public record UpdateFeaturePlanningCommand(
String featureCode,
String releaseCode,
LocalDate plannedCompletionDate,
FeaturePlanningStatus planningStatus,
String featureOwner,
Expand All @@ -69,7 +70,8 @@ public record UpdateFeaturePlanningCommand(
public record MoveFeatureToReleaseCommand(
String featureCode, String targetReleaseCode, String rationale, String movedBy) {}

public record RemoveFeatureFromReleaseCommand(String featureCode, String rationale, String removedBy) {}
public record RemoveFeatureFromReleaseCommand(
String featureCode, String releaseCode, String rationale, String removedBy) {}

/* Comment Commands */
public record CreateCommentCommand(String featureCode, String content, String createdBy) {}
Expand Down
3 changes: 3 additions & 0 deletions src/main/java/com/sivalabs/ft/features/domain/FeatureRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ interface FeatureRepository extends ListCrudRepository<Feature, Long> {
@Query("select f from Feature f left join fetch f.release where f.code = :code")
Optional<Feature> findByCode(String code);

@Query("select f from Feature f join fetch f.release r where f.code = :code and r.code = :releaseCode")
Optional<Feature> findByCodeAndReleaseCode(String code, String releaseCode);

@Query("select f from Feature f left join fetch f.release where f.release.code = :releaseCode")
List<Feature> findByReleaseCode(String releaseCode);

Expand Down
14 changes: 8 additions & 6 deletions src/main/java/com/sivalabs/ft/features/domain/FeatureService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,7 @@ public void assignFeatureToRelease(AssignFeatureToReleaseCommand cmd) {

@Transactional
public void updateFeaturePlanning(UpdateFeaturePlanningCommand cmd) {
Feature feature = featureRepository
.findByCode(cmd.featureCode())
.orElseThrow(() -> new ResourceNotFoundException("Feature not found: " + cmd.featureCode()));
Feature feature = findFeatureInRelease(cmd.featureCode(), cmd.releaseCode());

// Validate planning status transition
if (cmd.planningStatus() != null) {
Expand Down Expand Up @@ -345,9 +343,7 @@ public void moveFeatureToRelease(MoveFeatureToReleaseCommand cmd) {

@Transactional
public void removeFeatureFromRelease(RemoveFeatureFromReleaseCommand cmd) {
Feature feature = featureRepository
.findByCode(cmd.featureCode())
.orElseThrow(() -> new ResourceNotFoundException("Feature not found: " + cmd.featureCode()));
Feature feature = findFeatureInRelease(cmd.featureCode(), cmd.releaseCode());

String releaseCode = feature.getRelease() != null ? feature.getRelease().getCode() : "no release";

Expand All @@ -370,4 +366,10 @@ public void removeFeatureFromRelease(RemoveFeatureFromReleaseCommand cmd) {
cmd.removedBy(),
cmd.rationale());
}

private Feature findFeatureInRelease(String featureCode, String releaseCode) {
return featureRepository
.findByCodeAndReleaseCode(featureCode, releaseCode)
.orElseThrow(() -> new ResourceNotFoundException("Feature not found: " + featureCode));
}
}
119 changes: 111 additions & 8 deletions ...java/com/sivalabs/ft/features/api/controllers/ReleaseFeaturePlanningIntegrationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,51 @@ void shouldHandleFeatureNotFoundWhenUpdatingPlanning() {
assertThat(result).hasStatus(HttpStatus.NOT_FOUND);
}

@Test
@DisplayName("Should return 404 when updating planning through another product's release")
@WithMockOAuth2User(
username = "user",
roles = {"PRODUCT_MANAGER"})
void shouldReturn404WhenUpdatingPlanningThroughAnotherProductsRelease()
throws JsonMappingException, JsonProcessingException {
var updatePayload = """
{
"planningStatus": "IN_PROGRESS"
}
""";
var result = mvc.patch()
.uri("/api/releases/{releaseCode}/features/{featureCode}/planning", "RIDER-2024.2.6", "IDEA-1")
.contentType(MediaType.APPLICATION_JSON)
.content(updatePayload)
.exchange();
assertThat(result).hasStatus(HttpStatus.NOT_FOUND);
var feature = getFeatureFromRelease("IDEA-2023.3.8", "IDEA-1");
assertThat(feature.get("planningStatus")).isEqualTo("NOT_STARTED");
}

@Test
@DisplayName("Should return 404 when updating planning through different release of same product")
@WithMockOAuth2User(
username = "user",
roles = {"PRODUCT_MANAGER"})
void shouldReturn404WhenUpdatingPlanningThroughDifferentReleaseOfSameProduct()
throws JsonMappingException, JsonProcessingException {
var updatePayload =
"""
{
"notes": "Should not update through another IDEA release"
}
""";
var result = mvc.patch()
.uri("/api/releases/{releaseCode}/features/{featureCode}/planning", "IDEA-2024.2.3", "IDEA-1")
.contentType(MediaType.APPLICATION_JSON)
.content(updatePayload)
.exchange();
assertThat(result).hasStatus(HttpStatus.NOT_FOUND);
var feature = getFeatureFromRelease("IDEA-2023.3.8", "IDEA-1");
assertThat(feature.get("notes")).isEqualTo("Initial notes");
}

@Test
@DisplayName("Should reject invalid status transition from NOT_STARTED to DONE")
@WithMockOAuth2User(
Expand Down Expand Up @@ -951,30 +996,27 @@ void shouldRemoveFeatureFromRelease() throws JsonMappingException, JsonProcessin
}

@Test
@DisplayName("Should handle removal of unassigned feature gracefully")
@DisplayName("Should return 404 when removing unassigned feature from release")
@WithMockOAuth2User(
username = "testuser",
roles = {"ADMIN"})
void shouldHandleRemovalOfUnassignedFeatureGracefully() {
// Try to remove a feature that's not assigned to any release
void shouldReturn404WhenRemovingUnassignedFeatureFromRelease() {
var removePayload = """
{
"rationale": "Removing unassigned feature"
}
""";
var result = mvc.delete()
.uri("/api/releases/{releaseCode}/features/{featureCode}", "IDEA-2023.3.8", "IDEA-1")
.uri("/api/releases/{releaseCode}/features/{featureCode}", "IDEA-2023.3.8", "IDEA-3")
.contentType(MediaType.APPLICATION_JSON)
.content(removePayload)
.exchange();
// Should succeed even if feature wasn't assigned to this specific release
assertThat(result).hasStatus2xxSuccessful();
// Verify the feature is still unassigned (not in the release)
assertThat(result).hasStatus(HttpStatus.NOT_FOUND);
var getResult = mvc.get()
.uri("/api/releases/{releaseCode}/features", "IDEA-2023.3.8")
.exchange();
var responseBody = new String(getResult.getResponse().getContentAsByteArray(), StandardCharsets.UTF_8);
assertThat(responseBody).doesNotContain("IDEA-1");
assertThat(responseBody).doesNotContain("IDEA-3");
}

@Test
Expand Down Expand Up @@ -1061,6 +1103,67 @@ void shouldHandleFeatureNotFoundWhenRemoving() {
assertThat(result).hasStatus(HttpStatus.NOT_FOUND);
}

@Test
@DisplayName("Should return 404 when removing feature through another product's release")
@WithMockOAuth2User(
username = "testuser",
roles = {"ADMIN"})
void shouldReturn404WhenRemovingFeatureThroughAnotherProductsRelease()
throws JsonMappingException, JsonProcessingException {
var removePayload = """
{
"rationale": "Removing through another release"
}
""";
var result = mvc.delete()
.uri("/api/releases/{releaseCode}/features/{featureCode}", "RIDER-2024.2.6", "IDEA-1")
.contentType(MediaType.APPLICATION_JSON)
.content(removePayload)
.exchange();
assertThat(result).hasStatus(HttpStatus.NOT_FOUND);
var getResult = mvc.get()
.uri("/api/releases/{releaseCode}/features", "IDEA-2023.3.8")
.exchange();
var responseBody = new String(getResult.getResponse().getContentAsByteArray(), StandardCharsets.UTF_8);
List<Map<String, Object>> features = objectMapper.readValue(responseBody, new TypeReference<>() {});
var feature = features.stream()
.filter(f -> "IDEA-1".equals(f.get("code")))
.findFirst()
.orElseThrow();
assertThat(feature.get("planningStatus")).isEqualTo("NOT_STARTED");
}

@Test
@DisplayName("Should return 404 when removing feature through different release of same product")
@WithMockOAuth2User(
username = "testuser",
roles = {"ADMIN"})
void shouldReturn404WhenRemovingFeatureThroughDifferentReleaseOfSameProduct()
throws JsonMappingException, JsonProcessingException {
var removePayload =
"""
{
"rationale": "Removing through another IDEA release"
}
""";
var result = mvc.delete()
.uri("/api/releases/{releaseCode}/features/{featureCode}", "IDEA-2024.2.3", "IDEA-1")
.contentType(MediaType.APPLICATION_JSON)
.content(removePayload)
.exchange();
assertThat(result).hasStatus(HttpStatus.NOT_FOUND);
var getResult = mvc.get()
.uri("/api/releases/{releaseCode}/features", "IDEA-2023.3.8")
.exchange();
var responseBody = new String(getResult.getResponse().getContentAsByteArray(), StandardCharsets.UTF_8);
List<Map<String, Object>> features = objectMapper.readValue(responseBody, new TypeReference<>() {});
var feature = features.stream()
.filter(f -> "IDEA-1".equals(f.get("code")))
.findFirst()
.orElseThrow();
assertThat(feature.get("planningStatus")).isEqualTo("NOT_STARTED");
}

@Test
@DisplayName("Should validate required fields in assign feature payload")
@WithMockOAuth2User(
Expand Down
Loading