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.

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

Add "default" initialize property to QCBX #6037

Draft
wants to merge 12 commits into
base: production
Choose a base branch
from
+70 −34
Draft

Add "default" initialize property to QCBX #6037

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
09256f0
Add default Record ID property to QCBX
alesan99 Jan 3, 2025
ffe34d9
Lint code with ESLint and Prettier
alesan99 Jan 3, 2025
5418395
Add CURRENT_AGENT, CURRENT_USER, and BLANK
alesan99 Jan 3, 2025
698cf42
Merge branch 'issue-3994-2' of https://github.com/specify/specify7 in…
alesan99 Jan 3, 2025
b480976
Merge branch 'production' into issue-3994-2
alesan99 Jan 6, 2025
b1c2a52
Lint code with ESLint and Prettier
alesan99 Jan 6, 2025
41f21fe
Change "DefaultRecordId" to "default" (now a Record API Url)
alesan99 Jan 13, 2025
007ce67
Merge branch 'production' into issue-3994-2
alesan99 Jan 13, 2025
3036c93
Lint code with ESLint and Prettier
alesan99 Jan 13, 2025
d644da7
Merge branch 'production' into issue-3994-2
acwhite211 Jan 24, 2025
ed6c17b
Lint code with ESLint and Prettier
acwhite211 Jan 24, 2025
9875ee5
Merge branch 'production' into issue-3994-2
acwhite211 Jan 24, 2025
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
2 changes: 1 addition & 1 deletion specifyweb/frontend/js_src/lib/components/DataModel/resourceApi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,7 +902,7 @@ export const ResourceBase = Backbone.Model.extend({
});

// Check added to avoid infinite loop in following forEach for collectionRelationship see https://github.com/specify/specify7/issues/6025
if (self.specifyTable.name === 'CollectionRelationship') return json
if (self.specifyTable.name === 'CollectionRelationship') return json;

Object.entries(self.independentResources).forEach(
([fieldName, related]) => {
Expand Down
1 change: 1 addition & 0 deletions specifyweb/frontend/js_src/lib/components/FormEditor/viewSpec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -862,6 +862,7 @@ const queryComboBoxSpec = (
syncers.maybe(syncers.toBoolean),
syncers.default<boolean>(true)
),
defaultRecordId: syncers.xmlAttribute('initialize defaultRecordId', 'skip'),
Copy link
Contributor

Choose a reason for hiding this comment

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

(also a note, building on the above comment)

Whatever parsing behavior you implement in the form parsing code should also be reflected here in the Syncer as well (the Syncer is in charge of converting xml to json and vice-versa; for ease of use for visual editor).

One benefit of working with the Syncer is that it's also in charge of emitting errors to the user while they are using the CodeMirror text editor.

For example the following error when trying to render a to-many relationship with a QueryComboBox:

Screen.Recording.2025-01-07.at.3.20.14.AM.mov

Is caused because of the following console.error statement on the QueryComboBox syncer!:

specify7/specifyweb/frontend/js_src/lib/components/FormEditor/viewSpec.ts

Lines 820 to 836 in c55f4dd

field: pipe(
rawFieldSpec(table).field,
syncer(
({ parsed, ...rest }) => {
if (
parsed?.some(
(field) => field.isRelationship && relationshipIsToMany(field)
)
)
console.error(
'Unable to render a to-many relationship as a querycbx. Use a Subview instead'
);
return { parsed, ...rest };
},
(value) => value
)
),

Maybe you want to check the value: is one of the defaults, the default is being used correctly1, a valid number and a record exists with the ID, etc.

Footnotes

  1. We know CURRENT_AGENT and CURRENT_USER should only be used for relationships to Agent and Specifyuser (respectively): currently a user can technically use the defaults in any QueryComboBox ↩

legacyHelpContext: syncers.xmlAttribute('initialize hc', 'skip'),
// Make query compatible with multiple ORMs
legacyAdjustQuery: pipe(
Expand Down
2 changes: 2 additions & 0 deletions specifyweb/frontend/js_src/lib/components/FormFields/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,12 @@ const fieldRenderers: {
hasEditButton,
hasSearchButton,
hasViewButton,
defaultRecordId,
},
}) {
return field === undefined || !field.isRelationship ? null : (
<QueryComboBox
defaultRecordId={defaultRecordId}
field={field}
forceCollection={undefined}
formType={formType}
Expand Down
2 changes: 2 additions & 0 deletions specifyweb/frontend/js_src/lib/components/FormParse/fields.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export type FieldTypes = {
readonly hasViewButton: boolean;
readonly typeSearch: string | undefined;
readonly searchView: string | undefined;
readonly defaultRecordId: string | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

(optional)

If it's possible, can you include the respected default values (CURRENT_USER, CURRENT_AGENT, and BLANK) into this type?

That way, the default values will be present when using Intellisense with this property.
And arguably even more helpful, if the feature needs to be expanded/worked-on in the future, including the defaults in the type definition provides another clear pathway to investigate the feature, an alternative to searching the Code/GitHub/Speciforum for the property name alone.

}
>;
readonly Text: State<
Expand Down Expand Up @@ -246,6 +247,7 @@ const processFieldType: {
: getProperty('viewBtn')?.toLowerCase() === 'true',
typeSearch: getProperty('name'),
searchView: getProperty('searchView'),
defaultRecordId: getProperty('defaultRecordId'),
Copy link
Contributor

Choose a reason for hiding this comment

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

(just a note)

If desired, you can do whatever additional parsing here that you need!

For example, you can check if the value here is one of the defaults and if not, then try and parse it as an integer and otherwise return undefined.

That way, when working with defaultRecordId later down the line, you already know if it's supposed to be converted to a resource_uri, is one of the defaults, etc.

If you do change how the defaultRecordId is parsed, don't forget to modify the type definition accordingly and other places it's used (like the syncer).

};
} else {
console.error('QueryComboBox can only be used to display a relationship');
Expand Down
8 changes: 4 additions & 4 deletions specifyweb/frontend/js_src/lib/components/FormSliders/RecordSelectorFromCollection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,11 +86,11 @@ export function RecordSelectorFromCollection<SCHEMA extends AnySchema>({
isLazy &&
collection.related?.isNew() !== true &&
collection.models[index] === undefined
){
if (typeof handleFetch === 'function' ) {
handleFetch?.()
) {
if (typeof handleFetch === 'function') {
handleFetch?.();
} else {
void collection.fetch()
void collection.fetch();
}
}
}, [collection, isLazy, index, records.length, isToOne, handleFetch]);
Expand Down
1 change: 1 addition & 0 deletions specifyweb/frontend/js_src/lib/components/Forms/generateFormDefinition.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ function generateForm(
typeSearch: undefined,
searchView: undefined,
isReadOnly: mode === 'view',
defaultRecordId: undefined,
},
isRequired: false,
viewName: undefined,
Expand Down
1 change: 1 addition & 0 deletions specifyweb/frontend/js_src/lib/components/Merging/CompareField.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ function fieldToDefinition(
hasViewButton: false,
typeSearch: undefined,
searchView: undefined,
defaultRecordId: undefined,
};
else if (field.type === 'java.lang.Boolean')
return {
Expand Down
30 changes: 25 additions & 5 deletions specifyweb/frontend/js_src/lib/components/QueryComboBox/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ export function QueryComboBox({
typeSearch: initialTypeSearch,
forceCollection,
searchView,
defaultRecordId,
relatedTable: initialRelatedTable,
}: {
readonly id: string | undefined;
Expand All @@ -86,11 +87,32 @@ export function QueryComboBox({
readonly typeSearch: TypeSearch | string | undefined;
readonly forceCollection: number | undefined;
readonly searchView?: string;
readonly defaultRecordId?: string | undefined;
readonly relatedTable?: SpecifyTable | undefined;
}): JSX.Element {
React.useEffect(() => {
if (resource === undefined || !resource.isNew()) return;
if (field.name === 'cataloger') {
if (defaultRecordId !== undefined) {
let defaultUri: string | null;
if (defaultRecordId === 'CURRENT_AGENT') {
defaultUri = userInformation.agent.resource_uri;
} else if (defaultRecordId === 'CURRENT_USER') {
defaultUri = userInformation.resource_uri;
} else if (defaultRecordId === 'BLANK') {
defaultUri = null;
} else {
defaultUri = getResourceApiUrl(field.relatedTable.name, defaultRecordId);
}
alesan99 marked this conversation as resolved.
Show resolved Hide resolved

resource.set(
field.name,
resource.get(field.name) ?? defaultUri,
{
silent: true,
}
);
// The following cases can be technically be covered by the above code, but need to be kept for outdated forms
} else if (field.name === 'cataloger') {
alesan99 marked this conversation as resolved.
Show resolved Hide resolved
const record = toTable(resource, 'CollectionObject');
record?.set(
'cataloger',
Expand All @@ -99,8 +121,7 @@ export function QueryComboBox({
silent: true,
}
);
}
if (field.name === 'specifyUser') {
} else if (field.name === 'specifyUser') {
const record = toTable(resource, 'RecordSet');
record?.set(
'specifyUser',
Expand All @@ -109,8 +130,7 @@ export function QueryComboBox({
silent: true,
}
);
}
if (field.name === 'receivedBy') {
} else if (field.name === 'receivedBy') {
const record = toTable(resource, 'LoanReturnPreparation');
record?.set(
'receivedBy',
Expand Down
Loading