-
Notifications
You must be signed in to change notification settings - Fork 184
[jslib] Refactored Acknowledgements with Enhanced HTTP Utilities #9999
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
base: main
Are you sure you want to change the base?
Conversation
@@ -107,7 +107,7 @@ function array_find(array $array, callable $callback) | |||
$factory->database(), | |||
$factory->config(), | |||
[ | |||
__DIR__ . "/../project/", | |||
__DIR__ . "/../project/modules", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed to get things to load...
@@ -157,6 +157,7 @@ const resolve: webpack.ResolveOptions = { | |||
fallback: { | |||
fs: false, | |||
path: false, | |||
stream: require.resolve("stream-browserify"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm compile was giving errors without this
@@ -17,17 +17,20 @@ | |||
"jstat": "^1.9.5", | |||
"jszip": "^3.10.1", | |||
"papaparse": "^5.3.0", | |||
"pbkdf2": "^3.1.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
npm compile was giving errors without these
export class Client<T> { | ||
protected baseUrl: string; | ||
protected subEndpoint?: string; | ||
public getCustomMessage: ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is to allow each Client concrete class to define it's own errors across all the error types, not sure if it's the best implementation, but it seems to work — maybe it just needs a better name. return type be something like Record<string, string>
@@ -491,7 +494,6 @@ window.addEventListener('load', () => { | |||
document.getElementById('lorisworkspace') | |||
).render( | |||
<Index | |||
dataURL={`${loris.BaseURL}/acknowledgements/?format=json`} | |||
submitURL={`${loris.BaseURL}/acknowledgements/AcknowledgementsProcess`} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can also replace this submission call in this PR to show the 'create' functionality.
@driusan @ridz1208 would love your thoughts on this! Hoping to extend this to more parts of loris after and add more entities. I'm also curious what you think of the directory structure I've started to build out, as it will likely get expanded. Should everything live in jslib? I'm thinking the top level components that currently live in jsx/ should also get moved into what ever the top level javascript directory is and live under a |
This PR refactors the acknowledgements page to use a new, more robust and reusable API client and a suite of custom error classes. This change modernizes our data fetching approach by moving away from raw fetch requests and adopting a structured, type-safe methodology.
Key Changes
Introduced a new Core Library: Added a new directory at jslib/core containing:
Introduced a Entities Library: Added a new directory at jslib/entities containing:
Updated Acknowledgements Module
Misc