feat: enable filters config and scientific conditions on the backend #1337
Conversation
Ingvord
added
enhancement
| @UseGuards( | ||
| class ByPassAuthenticatedPoliciesGuard | ||
| extends PoliciesGuard | ||
| implements CanActivate | ||
| { | ||
| async canActivate(): Promise<boolean> { | ||
| return Promise.resolve(true); | ||
| } | ||
| }, | ||
| ) |
There was a problem hiding this comment.
If the endpoint should open to unauthenticated users, we can just use PoliciesGuard or remove UseGuards at all I suppose. Since we are not checking policies here
| operator: string; | ||
| } | ||
|
|
||
| export const kDefaultFilters: FilterConfig[] = [ |
There was a problem hiding this comment.
Sorry, old habit - roots from my C++ experience to follow Google code styling standards
k - prefix for a static const
I used to maintain JavaScriptMVC (pre-Angular SPA vanilla JS framework), together with the folks we adopted that standard also for JS
If that makes any sense
There was a problem hiding this comment.
Generally it would be more appreciated if the naming is consistent with existing naming convention within the application scope.
There was a problem hiding this comment.
#Sure, what is the current convention?
We don't have a strict naming convention in this project yet, but we should have one documented in the future.
Google also has a code styling standard for typescript as well, which, generally speaking is like unspoken standard for most of the typescript applications
There was a problem hiding this comment.
Thanks! I will check that out!
|
Sure, what is the current convention?
…On Wed, Aug 7, 2024, 11:37 Jay ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/users/schemas/user-settings.schema.ts
<#1337 (comment)>
:
> + | "TextFilterComponent";
+
+// Define the Filter interface
+export interface FilterConfig {
+ type: FilterComponentType;
+ visible: boolean;
+}
+
+// Define the Condition interface
+export interface ScientificCondition {
+ field: string;
+ value: string;
+ operator: string;
+}
+
+export const kDefaultFilters: FilterConfig[] = [
Generally it would be more appreciated if the naming is consistent with
existing naming convention within the application scope.
—
Reply to this email directly, view it on GitHub
<#1337 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACZDFHGS2EAP2XA66NMUSGTZQHTE7AVCNFSM6AAAAABLUIVO5SVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMRTG4ZTANZSGU>
.
You are receiving this because you were assigned.Message ID:
***@***.***>
|
This reverts commit 680317c.
|
@Junjiequan how can I fix that PR title check? Thanks |
|
@Ingvord The rules for PR title can be found in https://github.com/SciCatProject/scicat-backend-next/blob/master/.github/pr-title-checker-config.json |
Ingvord
changed the title
Ingvord
changed the title
|
Cool! That worked. Thanks! Also the whole thing looks a bit encryptic to me. Why do we need this? Is it possible to provide more descriptive error message? Thanks again for the help |
|
@Ingvord
I believe the error messages won't matter too much, it just takes a little time to accommodate the title rules. |
|
That's true if you do the project on a daily basis. Although if one only
rarely contributes something, they will have to accommodate with rules
every time 😆
…On Wed, Aug 21, 2024, 13:32 Jay ***@***.***> wrote:
@Ingvord <https://github.com/Ingvord>
It helps with mainly two things:
1. PR title prefix makes it easy to understand what kind of changes
will be included in the PR.
2. Helps with complete & well organized release notes.
I believe the error messages won't matter too much, it just takes a little
time to accommodate the title rules.
—
Reply to this email directly, view it on GitHub
<#1337 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACZDFHGKBJTVNGMOXETE3A3ZSR3GHAVCNFSM6AAAAABLUIVO5SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBRHAZDSOJRGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This one is to resolve #604
defaultendpoints