Services: Records: Raise CommunityNotSelectedError when community is … #288
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sakshamarora1
force-pushed
the
feature/require_community
branch
from
9c6b123 to
eec0529
Compare
kpsherva
reviewed
Sep 17, 2024
Comment on lines
+382
to
+386
| if ( | ||
| current_app.config["RDM_RECORD_ALWAYS_IN_COMMUNITY"] | ||
| and len(draft.parent.communities.ids) == 0 | ||
| ): | ||
| if not self.check_permission(identity, "publish", record=draft): |
There was a problem hiding this comment.
to improve the readability:
Suggested change
| if ( | |
| current_app.config["RDM_RECORD_ALWAYS_IN_COMMUNITY"] | |
| and len(draft.parent.communities.ids) == 0 | |
| ): | |
| if not self.check_permission(identity, "publish", record=draft): | |
| community_required = current_app.config["RDM_RECORD_ALWAYS_IN_COMMUNITY"] | |
| community_missing = len(draft.parent.communities.ids) == 0 | |
| can_publish = self.check_permission(identity, "publish", record=draft) | |
| if ( | |
| community_required and community_missing and not can_publish | |
| ): |
also not ideal, in some cases we check the permissions for publish twice (second time in line 388)
kpsherva
reviewed
Sep 17, 2024
| # If config is True and there are no communities selected | ||
| # Then, check for permissions to upload without community | ||
| if ( | ||
| current_app.config["RDM_RECORD_ALWAYS_IN_COMMUNITY"] |
There was a problem hiding this comment.
the configuration variable is declared in rdm-records, therefore this module knows nothing about it if they are not installed together.
In addition, drafts resources does not know anything about invenio-communities, therefore this check should be done in a place that is aware of both drafts and communities
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes: CERNDocumentServer/cds-rdm#88