-
Notifications
You must be signed in to change notification settings - Fork 433
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
Fixes for #2891 #2889 #2897
Fixes for #2891 #2889 #2897
Conversation
@atarix83 The suggestion from CodeQL is to turn LazyDataServicesMap into an actual Map object: in export const LAZY_DATA_SERVICES = new Map<string, () => Promise<Type<HALDataService<any>>>>()
LAZY_DATA_SERVICES.set(AUTHORIZATION.value, () => import('./data/feature-authorization/authorization-data.service').then(m => m.AuthorizationDataService));
... in lazy-service pass both the map and the key, so you can write something like: if (map.has(key)) {
const loader = map.get(key):
loader();
} else {
trow new Error(`The key ${key} was not found in the map ${map}`);
} where map has the type |
thanks @artlowel for suggestions, now every checks pass and PR is to be reviewed |
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.
👍 Thanks @atarix83 ! I've reviewed the code and it looks reasonable. CodeQL also now approves it. I've also tested to make sure that no obvious bugs exist... I could not find any issues & I'm able to verify that the CSRF token is still working as expected.
References
Description
This is a small follow-up PR to fix the mentioned issues
Checklist
yarn lint
yarn check-circ-deps
)package.json
), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.