feat(forms): add form support with field types and access control#13
feat(forms): add form support with field types and access control#13nebay-abraha merged 2 commits intomainfrom
Conversation
- Update version to 1.99.1
There was a problem hiding this comment.
Pull request overview
This PR introduces client-side model support for the Forms API (form definitions, field polymorphism, and access control), along with tests/resources, and bumps the library version to 1.99.1.
Changes:
- Added new form-related response models (
Form,FormInfo,FormFieldhierarchy,AccessControl,FormState) with Jackson polymorphic field handling. - Added JUnit tests and JSON fixtures covering form/form-search and several field types.
- Updated
pom.xmlversion and documented the additions inCHANGELOG.md.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/resources/Form.json | Adds a form fixture used for deserialization testing. |
| src/test/java/com/researchspace/api/clientmodel/FormTest.java | Tests Form deserialization, builder defaults, and addField chaining. |
| src/test/java/com/researchspace/api/clientmodel/FormInfoTest.java | Tests FormInfo deserialization from FormSearchResult and constructors/builders. |
| src/test/java/com/researchspace/api/clientmodel/FormFieldTest.java | Tests Jackson polymorphic serialization/deserialization for several FormField subtypes. |
| src/test/java/com/researchspace/api/clientmodel/AccessControlTest.java | Tests AccessControl builder/constructors and basic Jackson serialization. |
| src/main/java/com/researchspace/api/clientmodel/TimeFormField.java | Adds a time field subtype. |
| src/main/java/com/researchspace/api/clientmodel/TextFormField.java | Adds a long-text field subtype. |
| src/main/java/com/researchspace/api/clientmodel/StringFormField.java | Adds a short-text field subtype. |
| src/main/java/com/researchspace/api/clientmodel/RadioFormField.java | Adds a radio (single-choice) field subtype. |
| src/main/java/com/researchspace/api/clientmodel/NumberFormField.java | Adds a numeric field subtype with constraints. |
| src/main/java/com/researchspace/api/clientmodel/FormState.java | Adds a form publishing state enum. |
| src/main/java/com/researchspace/api/clientmodel/FormInfo.java | Extends FormInfo to include state/access-control/tags/icon metadata. |
| src/main/java/com/researchspace/api/clientmodel/FormField.java | Introduces base polymorphic FormField abstraction and subtype mapping. |
| src/main/java/com/researchspace/api/clientmodel/Form.java | Adds full Form model including a list of fields and addField. |
| src/main/java/com/researchspace/api/clientmodel/DateFormField.java | Adds a date field subtype with ISO serialization. |
| src/main/java/com/researchspace/api/clientmodel/ChoiceFormField.java | Adds a checkbox/multi-choice field subtype. |
| src/main/java/com/researchspace/api/clientmodel/AccessControl.java | Adds an access control/permissions model. |
| pom.xml | Bumps artifact version to 1.99.1. |
| CHANGELOG.md | Documents new form support and FormInfo enhancements. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import java.time.Instant; | ||
|
|
There was a problem hiding this comment.
There is an unused import (java.time.Instant). If the intent was to expose lastModified as an Instant, consider switching the field type accordingly; otherwise, remove the unused import to keep the file clean.
| import java.time.Instant; |
| assertNotNull(dateField.getMax()); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
FormFieldTest covers polymorphic (de)serialization for String/Number/Choice/Date, but the PR also introduces RadioFormField, TextFormField, and TimeFormField without similar coverage. Adding serialization + deserialization assertions for these types would help catch issues with the @JsonSubTypes configuration and field-specific properties (defaultOption, long defaultValue, etc.).
| @Test | |
| @Test | |
| void testRadioFormFieldSerialization() throws JsonProcessingException { | |
| RadioFormField field = new RadioFormField("Test Radio Field", | |
| Arrays.asList("Option A", "Option B", "Option C"), | |
| "Option B"); | |
| field.setId(5L); | |
| field.setIndex(4); | |
| String json = objectMapper.writeValueAsString(field); | |
| assertNotNull(json); | |
| assertTrue(json.contains("\"type\":\"Radio\"")); | |
| assertTrue(json.contains("\"defaultOption\":\"Option B\"")); | |
| FormField deserialized = objectMapper.readValue(json, FormField.class); | |
| assertInstanceOf(RadioFormField.class, deserialized); | |
| RadioFormField radioField = (RadioFormField) deserialized; | |
| assertEquals("Test Radio Field", radioField.getName()); | |
| assertEquals(3, radioField.getOptions().size()); | |
| assertEquals("Option B", radioField.getDefaultOption()); | |
| } | |
| @Test | |
| void testTextFormFieldSerialization() throws JsonProcessingException { | |
| TextFormField field = new TextFormField("Test Text Field", "default text"); | |
| field.setId(6L); | |
| field.setIndex(5); | |
| String json = objectMapper.writeValueAsString(field); | |
| assertNotNull(json); | |
| assertTrue(json.contains("\"type\":\"Text\"")); | |
| assertTrue(json.contains("\"defaultValue\":\"default text\"")); | |
| FormField deserialized = objectMapper.readValue(json, FormField.class); | |
| assertInstanceOf(TextFormField.class, deserialized); | |
| TextFormField textField = (TextFormField) deserialized; | |
| assertEquals("Test Text Field", textField.getName()); | |
| assertEquals("default text", textField.getDefaultValue()); | |
| } | |
| @Test | |
| void testTimeFormFieldSerialization() throws JsonProcessingException { | |
| long defaultTime = 3600000L; // 01:00:00.000 in milliseconds | |
| long minTime = 0L; // 00:00:00.000 | |
| long maxTime = 86400000L; // 24:00:00.000 | |
| TimeFormField field = new TimeFormField("Test Time Field", defaultTime, minTime, maxTime); | |
| field.setId(7L); | |
| field.setIndex(6); | |
| String json = objectMapper.writeValueAsString(field); | |
| assertNotNull(json); | |
| assertTrue(json.contains("\"type\":\"Time\"")); | |
| assertTrue(json.contains("\"defaultValue\":" + defaultTime)); | |
| FormField deserialized = objectMapper.readValue(json, FormField.class); | |
| assertInstanceOf(TimeFormField.class, deserialized); | |
| TimeFormField timeField = (TimeFormField) deserialized; | |
| assertEquals("Test Time Field", timeField.getName()); | |
| assertEquals(defaultTime, timeField.getDefaultValue()); | |
| assertEquals(minTime, timeField.getMin()); | |
| assertEquals(maxTime, timeField.getMax()); | |
| } | |
| @Test |
CHANGELOG.md
Outdated
| - `FormState` enum - Enumeration for form publishing states (NEW, PUBLISHED, UNPUBLISHED, OLD) | ||
| - `ChoiceFormField` model - Checkbox field with multiple selection support | ||
| - `DateFormField` model - Date input field with min/max validation | ||
| - `NumberFormField` model - Numeric field with range and decimal place |
There was a problem hiding this comment.
Changelog entry has a small grammar issue: “Numeric field with range and decimal place” reads incomplete/incorrect (should be “decimal places”). Please adjust the wording for clarity.
| - `NumberFormField` model - Numeric field with range and decimal place | |
| - `NumberFormField` model - Numeric field with range and decimal places |
| <artifactId>rspace-client-java-model</artifactId> | ||
| <version>1.99.0</version> | ||
| <version>1.99.1</version> | ||
| <name>rspace-client-java-model</name> |
There was a problem hiding this comment.
PR description mentions only the version bump, but this PR also introduces new Form/Field/AccessControl models and multiple new tests/resources. Please update the PR description to reflect the functional scope so reviewers/release notes are accurate.
| public abstract class FormField extends IdentifiableNameable { | ||
|
|
||
| @JsonProperty("lastModified") | ||
| private String lastModifiedMillis; |
There was a problem hiding this comment.
lastModified is represented as an epoch-millis number in the test JSON (e.g., Form.json), but the model maps it to a String (lastModifiedMillis). This will coerce numbers to strings on deserialization and will serialize back as a JSON string, which is likely not what the API returns/expects. Consider changing this to a Long (or Date/Instant with appropriate Jackson config) and renaming the field to match its semantics (e.g., lastModified).
| private String lastModifiedMillis; | |
| private Long lastModifiedMillis; |
- fix typo - update test
Uh oh!
There was an error while loading. Please reload this page.