Skip to content

feat: default access form selection implementation #1127

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
ChrisiSailer wants to merge 5 commits into
base: master
Choose a base branch
from
+210 −6
Open

feat: default access form selection implementation #1127

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cc90a03
feat: default access form selection implementation
ChrisiSailer Mar 27, 2026
1076f82
fix: changed tests
ChrisiSailer Mar 27, 2026
d298190
fix: implemented suggested changes and handeled null form value
ChrisiSailer Mar 30, 2026
0392c4a
refactor: implemented requested changes
ChrisiSailer Apr 27, 2026
377a5db
Merge branch 'master' into fix/solve_access_form_assignment_to_new_re…
stetsche May 11, 2026
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
12 changes: 12 additions & 0 deletions backend/src/main/java/eu/bbmri_eric/negotiator/form/repository/AccessFormRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,16 @@ public interface AccessFormRepository extends JpaRepository<AccessForm, Long> {
+ ")",
nativeQuery = true)
boolean isElementPartOfSectionOfAccessForm(Long accessFormId, Long sectionId, Long elementId);

@Query(
value =
"SELECT a.* "
+ "FROM resource r "
+ " INNER JOIN access_form a on a.id = r.access_form_id "
+ "WHERE r.organization_id = :organizationId "
+ "GROUP BY a.id "
+ "ORDER BY COUNT(r.id) DESC "
+ "LIMIT 1",
nativeQuery = true)
Optional<AccessForm> findFirstMostCommonAccessFormByOrganization(Long organizationId);
Copy link
Copy Markdown
Collaborator

@stetsche stetsche Apr 27, 2026

Choose a reason for hiding this comment

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

Some unit tests for the repository to test more cases then covered in the integration test would be great, but not a blocker.

}
22 changes: 18 additions & 4 deletions backend/src/main/java/eu/bbmri_eric/negotiator/governance/resource/ResourceServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ public class ResourceServiceImpl implements ResourceService {
private final OrganizationRepository organizationRepository;
private final NegotiationAccessManager negotiationAccessManager;

private static final long DEFAULT_ACCESS_FORM_ID = 1L;

public ResourceServiceImpl(
NetworkRepository networkRepository,
ResourceRepository repository,
Expand Down Expand Up @@ -216,10 +218,22 @@ public List<ResourceResponseModel> addResources(List<ResourceCreateDTO> resource
.findById(resDTO.getDiscoveryServiceId())
.orElseThrow(
() -> new DiscoveryServiceNotFoundException(resDTO.getDiscoveryServiceId()));
AccessForm accessForm =
accessFormRepository
.findById(resDTO.getAccessFormId())
.orElseThrow(() -> new AccessFormNotFoundException(resDTO.getAccessFormId()));
AccessForm accessForm;
if (resDTO.getAccessFormId() == null) {
accessForm =
accessFormRepository
.findFirstMostCommonAccessFormByOrganization(resDTO.getOrganizationId())
.orElse(
Comment thread
stetsche marked this conversation as resolved.
Outdated
accessFormRepository
.findById(DEFAULT_ACCESS_FORM_ID)
.orElseThrow(
() -> new AccessFormNotFoundException(DEFAULT_ACCESS_FORM_ID)));
} else {
accessForm =
Copy link
Copy Markdown
Collaborator

@stetsche stetsche Apr 9, 2026

Choose a reason for hiding this comment

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

If we could try to reduce the queries to the database that would be great as well. This logic could be in a loop of lots of resources with a relatively small amount of access forms and organizations, but they are queried all the time. I see that this behavior was preexisting and we could address it in a separate issue as well.
Code for improving the performance here could look something like:

...
    Map<Long, AccessForm> accessFormById = new HashMap<>();
    Map<Long, AccessForm> fallbackAccessFormByOrganization = new HashMap<>();
    Map<Long, DiscoveryService> discoveryServiceById = new HashMap<>();

    for (ResourceCreateDTO resDTO : resourcesCreateDTO) {
      DiscoveryService discoveryService = discoveryServiceById.computeIfAbsent(resDTO.getDiscoveryServiceId(),
          id ->
            discoveryServiceRepository
                .findById(id)
                .orElseThrow(
                    () -> new DiscoveryServiceNotFoundException(id)));
      AccessForm accessForm;
      if (resDTO.getAccessFormId() == null) {
        accessForm = fallbackAccessFormByOrganization.computeIfAbsent(
              resDTO.getOrganizationId(),
              orgId ->
                  accessFormRepository
                      .findFirstMostCommonAccessFormByOrganization(orgId)
                      .orElseGet(() -> getAccessFormById(accessFormById, DEFAULT_ACCESS_FORM_ID));
      } else {
        accessForm = getAccessFormById(accessFormById, resDTO.getAccessFormId());
      }
...

accessFormRepository
.findById(resDTO.getAccessFormId())
.orElseThrow(() -> new AccessFormNotFoundException(resDTO.getAccessFormId()));
}
Organization organization =
organizationRepository
.findById(resDTO.getOrganizationId())
Expand Down
3 changes: 1 addition & 2 deletions ...end/src/main/java/eu/bbmri_eric/negotiator/governance/resource/dto/ResourceCreateDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ public class ResourceCreateDTO {
@Schema(description = "ID of the associated organization", example = "1")
private Long organizationId;

@NotNull
@Schema(description = "ID of the access form related to the resource", example = "42")
@Schema(description = "Optional ID of the access form related to the resource", example = "42")
private Long accessFormId;

@NotNull
Expand Down
4 changes: 4 additions & 0 deletions ...d/src/main/java/eu/bbmri_eric/negotiator/governance/resource/dto/ResourceWithRepsDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package eu.bbmri_eric.negotiator.governance.resource.dto;

import eu.bbmri_eric.negotiator.form.dto.AccessFormDTO;
import eu.bbmri_eric.negotiator.user.UserDTO;
import io.swagger.v3.oas.annotations.media.Schema;
import java.util.Set;
Expand All @@ -14,4 +15,7 @@ public class ResourceWithRepsDTO extends ResourceResponseModel {
example =
"[{\"id\": \"123\", \"name\": \"Sarah Rep\", \"email\": \"sarah@example.com\"}, {\"id\": \"456\", \"name\": \"Adam Rep\", \"email\": \"adam@example.com\"}]")
private Set<UserDTO> representatives;

@Schema(description = "Access form associated with this resource")
private AccessFormDTO accessForm;
Comment thread
stetsche marked this conversation as resolved.
}
161 changes: 161 additions & 0 deletions ...d/src/test/java/eu/bbmri_eric/negotiator/governance/resource/ResourceControllerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,10 @@ public class ResourceControllerTests {

private static final String RESOURCES_ENDPOINT = "/v3/resources";

private static final long DEFAULT_ACCESS_FORM_ID = 1L;

private static final long DEFAULT_DISCOVERY_SERVICE_ID = 1L;

@Autowired private WebApplicationContext context;
@Autowired private ResourceRepository repository;
@Autowired private OrganizationRepository organizationRepository;
Expand All @@ -71,6 +75,36 @@ public void before() {
mockMvc = MockMvcBuilders.webAppContextSetup(context).apply(springSecurity()).build();
}

private Organization createOrganization(String suffix) {
return Organization.builder()
.name("Organization " + suffix)
.description("Organization " + suffix)
.externalId("test_organization_" + suffix)
.build();
}

private ResourceCreateDTO createResource(
String name, String sourceId, Long organizationId, Long accessFormId) {
return ResourceCreateDTO.builder()
.name(name)
.description(name)
.contactEmail(name.toLowerCase().replace(" ", "") + "@test.org")
.uri("https://" + name.toLowerCase().replace(" ", "") + ".test.org")
.sourceId(sourceId)
.organizationId(organizationId)
.accessFormId(accessFormId)
.discoveryServiceId(DEFAULT_DISCOVERY_SERVICE_ID)
.build();
}

private DiscoveryService createDiscoveryService(long id) {
return DiscoveryService.builder()
.name("test_discovery_service")
.id(id)
.url("http://discoveryservice.net")
.build();
}

@Test
@WithMockUser("researcher")
void getResourceById_validId_ok() throws Exception {
Expand Down Expand Up @@ -427,6 +461,133 @@ void addResource_batchOfResources_ok() throws Exception {
assertEquals(resourceDTO2.getUri(), resource2.get().getUri());
}

@Test
@WithUserDetails("admin")
void addResource_noAccessFormSelected_ok() throws Exception {
Organization org1 = createOrganization("10");
organizationRepository.save(org1);
Long org1Id = organizationRepository.findByExternalId("test_organization_10").get().getId();

ResourceCreateDTO resourceDTO1 = createResource("Resource 10", "resource_10", org1Id, null);

discoveryServiceRepository
.findById(DEFAULT_DISCOVERY_SERVICE_ID)
.orElseGet(
() ->
discoveryServiceRepository.save(
createDiscoveryService(DEFAULT_DISCOVERY_SERVICE_ID)));

String requestBody = TestUtils.jsonFromRequest(Arrays.asList(resourceDTO1));
MvcResult result =
mockMvc
.perform(
MockMvcRequestBuilders.post(RESOURCES_ENDPOINT)
.contentType(MediaType.APPLICATION_JSON)
.content(requestBody))
.andExpect(status().isCreated())
.andExpect(content().contentType("application/hal+json"))
.andExpect(jsonPath("$._embedded.resources[0].name", is(resourceDTO1.getName())))
.andExpect(
jsonPath("$._embedded.resources[0].sourceId", is(resourceDTO1.getSourceId())))
.andExpect(
jsonPath("$._embedded.resources[0].description", is(resourceDTO1.getDescription())))
.andExpect(
jsonPath(
"$._embedded.resources[0].contactEmail", is(resourceDTO1.getContactEmail())))
.andExpect(jsonPath("$._embedded.resources[0].uri", is(resourceDTO1.getUri())))
.andReturn();

String id1 =
JsonPath.parse(result.getResponse().getContentAsString())
.read("$._embedded.resources[0].sourceId");
Optional<Resource> resource1 = repository.findBySourceId(id1);
assert resource1.isPresent();
assertEquals(resourceDTO1.getName(), resource1.get().getName());
assertEquals(resourceDTO1.getDescription(), resource1.get().getDescription());
Comment thread
stetsche marked this conversation as resolved.
assertEquals(resourceDTO1.getContactEmail(), resource1.get().getContactEmail());
assertEquals(resourceDTO1.getUri(), resource1.get().getUri());
assertEquals(DEFAULT_ACCESS_FORM_ID, resource1.get().getAccessForm().getId());
}

@Test
@WithUserDetails("admin")
void addResource_useOrganizationDefaultAccessForm_ok() throws Exception {
Organization org1 = createOrganization("11");
organizationRepository.save(org1);
Long org1Id = organizationRepository.findByExternalId("test_organization_11").get().getId();

ResourceCreateDTO resourceDTO1 = createResource("Resource 11", "resource_11", org1Id, 2L);
ResourceCreateDTO resourceDTO2 = createResource("Resource 12", "resource_12", org1Id, null);

discoveryServiceRepository
.findById(DEFAULT_DISCOVERY_SERVICE_ID)
.orElseGet(
() ->
discoveryServiceRepository.save(
createDiscoveryService(DEFAULT_DISCOVERY_SERVICE_ID)));

String requestBody1 = TestUtils.jsonFromRequest(Arrays.asList(resourceDTO1));
MvcResult result1 =
mockMvc
.perform(
MockMvcRequestBuilders.post(RESOURCES_ENDPOINT)
.contentType(MediaType.APPLICATION_JSON)
.content(requestBody1))
.andExpect(status().isCreated())
.andExpect(content().contentType("application/hal+json"))
.andExpect(jsonPath("$._embedded.resources[0].name", is(resourceDTO1.getName())))
.andExpect(
jsonPath("$._embedded.resources[0].sourceId", is(resourceDTO1.getSourceId())))
.andExpect(
jsonPath("$._embedded.resources[0].description", is(resourceDTO1.getDescription())))
.andExpect(
jsonPath(
"$._embedded.resources[0].contactEmail", is(resourceDTO1.getContactEmail())))
.andExpect(jsonPath("$._embedded.resources[0].uri", is(resourceDTO1.getUri())))
.andReturn();

String requestBody2 = TestUtils.jsonFromRequest(Arrays.asList(resourceDTO2));
MvcResult result2 =
mockMvc
.perform(
MockMvcRequestBuilders.post(RESOURCES_ENDPOINT)
.contentType(MediaType.APPLICATION_JSON)
.content(requestBody2))
.andExpect(status().isCreated())
.andExpect(content().contentType("application/hal+json"))
.andExpect(jsonPath("$._embedded.resources[0].name", is(resourceDTO2.getName())))
.andExpect(
jsonPath("$._embedded.resources[0].sourceId", is(resourceDTO2.getSourceId())))
.andExpect(
jsonPath("$._embedded.resources[0].description", is(resourceDTO2.getDescription())))
.andExpect(
jsonPath(
"$._embedded.resources[0].contactEmail", is(resourceDTO2.getContactEmail())))
.andExpect(jsonPath("$._embedded.resources[0].uri", is(resourceDTO2.getUri())))
.andReturn();

String id1 =
JsonPath.parse(result1.getResponse().getContentAsString())
.read("$._embedded.resources[0].sourceId");
Optional<Resource> resource1 = repository.findBySourceId(id1);
assert resource1.isPresent();
assertEquals(resourceDTO1.getName(), resource1.get().getName());
assertEquals(resourceDTO1.getDescription(), resource1.get().getDescription());
assertEquals(resourceDTO1.getContactEmail(), resource1.get().getContactEmail());
assertEquals(resourceDTO1.getUri(), resource1.get().getUri());

Long id2 =
JsonPath.parse(result2.getResponse().getContentAsString())
.read("$._embedded.resources[0].id", Long.class);
Optional<Resource> resource2 = repository.findById(id2);
assert resource2.isPresent();
assertEquals(resourceDTO2.getName(), resource2.get().getName());
assertEquals(resourceDTO2.getDescription(), resource2.get().getDescription());
Comment thread
stetsche marked this conversation as resolved.
assertEquals(resourceDTO2.getContactEmail(), resource2.get().getContactEmail());
assertEquals(resourceDTO2.getUri(), resource2.get().getUri());
assertEquals(2L, resource2.get().getAccessForm().getId());
}

@Test
@WithUserDetails("admin")
void updateResource_singleResource_ok() throws Exception {
Expand Down
13 changes: 13 additions & 0 deletions frontend/src/components/OrganizationListItem.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,19 @@
</span>
</small>
</div>

<!-- Access Form -->
<div class="access-form">
<small class="text-muted">
<strong>Access Form: </strong>
<span v-if="resource.accessForm">
{{ resource.accessForm.name }}
</span>
<span v-else class="text-warning">
None assigned
</span>
</small>
</div>
</div>

<!-- Resource Status Icon -->
Expand Down
12 changes: 3 additions & 9 deletions frontend/src/components/governance/CreateResourceModal.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,13 @@

<div class="mb-3">
<label for="accessForm" class="form-label">
Access Form <span class="text-danger">*</span>
Access Form
Comment thread
stetsche marked this conversation as resolved.
Outdated
</label>
<select
id="accessForm"
v-model="formData.accessFormId"
class="form-select"
:class="{ 'is-invalid': errors.accessFormId }"
required
>
<option value="">Select an access form</option>
<option v-for="form in accessForms" :key="form.id" :value="form.id">
Expand Down Expand Up @@ -232,13 +231,12 @@
const hasName = formData.value.name?.trim().length > 0
const hasDescription = formData.value.description?.trim().length > 0
const hasSourceId = formData.value.sourceId?.trim().length > 0
const hasAccessForm = formData.value.accessFormId && formData.value.accessFormId !== ''
const hasDiscoveryService =
formData.value.discoveryServiceId &&
formData.value.discoveryServiceId !== '' &&
!isNaN(Number(formData.value.discoveryServiceId))

return hasName && hasDescription && hasSourceId && hasAccessForm && hasDiscoveryService
return hasName && hasDescription && hasSourceId && hasDiscoveryService
})
Comment thread
ChrisiSailer marked this conversation as resolved.

// Watch for organization ID changes
Expand Down Expand Up @@ -277,10 +275,6 @@
errors.value.sourceId = 'Source ID is required'
}

if (!formData.value.accessFormId) {
errors.value.accessFormId = 'Access form is required'
}

if (!formData.value.discoveryServiceId) {
errors.value.discoveryServiceId = 'Discovery service ID is required'
} else if (isNaN(formData.value.discoveryServiceId) || formData.value.discoveryServiceId < 1) {
Expand Down Expand Up @@ -345,7 +339,7 @@
description: formData.value.description.trim(),
sourceId: formData.value.sourceId.trim(),
organizationId: formData.value.organizationId,
accessFormId: Number(formData.value.accessFormId),
accessFormId: formData.value.accessFormId !== '' ? Number(formData.value.accessFormId) : null,

Check warning on line 342 in frontend/src/components/governance/CreateResourceModal.vue

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Unexpected negated condition. 


See more on https://sonarcloud.io/project/issues?id=bbmri-eric_negotiator-frontend&issues=AZ09S50VBfEcQZ3_edSv&open=AZ09S50VBfEcQZ3_edSv&pullRequest=1127
discoveryServiceId: Number(formData.value.discoveryServiceId),
contactEmail: formData.value.contactEmail?.trim() || null,
uri: formData.value.uri?.trim() || null,
Expand Down
Loading