moderation: add 'verified' field to records #1399
Conversation
alejandromumo
force-pushed
the
add_record_verified
branch
from
e39b243
to
a0a7219
Compare
| return False | ||
| is_verified = ( | ||
| owner.resolve().verified_at is not None | ||
| ) # TODO property is_verified in user |
There was a problem hiding this comment.
WIP: Ideally, we want a computed property "is_verified" on the user
However, the owner.resolve() returns a User (model) instance. Therefore, verified is not a property we should be adding to the model. Perhaps we can resolve the user's API class instead?
Shelved it for now, unless there's a strong reason to do it now.
alejandromumo
changed the title
alejandromumo
force-pushed
the
add_record_verified
branch
from
3927414
to
3abcfd4
Compare
| @@ -126,6 +133,16 @@ class RDMSearchOptions(SearchOptions, SearchOptionsMixin): | |||
| "access_status": facets.access_status, | |||
| } | |||
|
|
|||
| # TODO we could do SearchOptions.params_interpreters_cls | |||
There was a problem hiding this comment.
Since I wanted to replace SortParam by VerifiedRecordsSortParam, I rewrote the params interpreters list.
I don't like this because:
- If we change the record's default list, it won't be picked up by RDM records (this happens with other record types, e..g communities, but I feel rdm records is special)
However, I did not like using SearchOptions.params_interpreters_cls because it would execute apply for both SortParam and VerifiedRecordsSortParam
There was a problem hiding this comment.
We have a similar problem with service components: if you change them via config, you might break functionalities.
At some point, we discussed that we should have a kind of "private" components, and to override them is not simply via a config var, but via a method that will explicitly list them somehow, a guard to warn you: Hey, do you know what you are doing? We didn't do it yet.
In components, we have a comment # Order of components are important!: you might want to add something similar, we another sentence.
alejandromumo
force-pushed
the
add_record_verified
branch
2 times, most recently
from
24f188b
to
b5086fe
Compare
alejandromumo
force-pushed
the
add_record_verified
branch
4 times, most recently
from
9b96f5b
to
e9383c1
Compare
There was a problem hiding this comment.
question: I am not entirely sure about adding the mapping to drafts as well.
There was a problem hiding this comment.
The is_verified is a calculated system field, which will not be part of the record saved in the db. Or am I wrong? You might don't need to add it to the various db schemas. It is probably necessary for OpenSearch mappings.
There was a problem hiding this comment.
Indeed it won't be saved in the DB.
However, it is currently being indexed also for drafts. Otherwise, other tests fail for drafts.
I think it is not necessary to have the field in drafts, I need to check how to only add it for the records on indexing.
There was a problem hiding this comment.
I had a closer look.
is_verified is a field on the parent, which is a common field for both records and drafts.
I don't see a problem with having is_verified on the draft since it won't be used (at least for now).
WDYT @ntarocco
There was a problem hiding this comment.
The is_verified is a calculated system field, which will not be part of the record saved in the db. Or am I wrong? You might don't need to add it to the various db schemas. It is probably necessary for OpenSearch mappings.
| @@ -126,6 +133,16 @@ class RDMSearchOptions(SearchOptions, SearchOptionsMixin): | |||
| "access_status": facets.access_status, | |||
| } | |||
|
|
|||
| # TODO we could do SearchOptions.params_interpreters_cls | |||
There was a problem hiding this comment.
We have a similar problem with service components: if you change them via config, you might break functionalities.
At some point, we discussed that we should have a kind of "private" components, and to override them is not simply via a config var, but via a method that will explicitly list them somehow, a guard to warn you: Hey, do you know what you are doing? We didn't do it yet.
In components, we have a comment # Order of components are important!: you might want to add something similar, we another sentence.
alejandromumo
force-pushed
the
add_record_verified
branch
from
e9383c1
to
7382234
Compare
alejandromumo
force-pushed
the
add_record_verified
branch
from
7382234
to
1533207
Compare
closes #1398
NEEDS: