Skip to content
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.

Sign up for GitHub

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

Votes consistent permissions #1724

Merged
merged 5 commits into from
Dec 4, 2024
Merged

Votes consistent permissions #1724

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1e9a40a
Use isAdmin for Dashboards upVote
dlpzx Nov 27, 2024
e3ba1e9
Disable upVote for non admins. Implement BE checks in upvote
dlpzx Nov 27, 2024
94eebc0
Disabled prop for vote button
dlpzx Dec 4, 2024
391dfbf
Dummy change
dlpzx Dec 4, 2024
97cc68a
Merge remote-tracking branch 'origin/main' into feat/votes-consistent…
dlpzx Dec 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion backend/dataall/modules/dashboards/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ def __init__(self):
)
)

add_vote_type('dashboard', DashboardIndexer)
add_vote_type('dashboard', DashboardIndexer, GET_DASHBOARD)

EnvironmentResourceManager.register(DashboardRepository())
log.info('Dashboard API has been loaded')
Expand Down
2 changes: 1 addition & 1 deletion backend/dataall/modules/redshift_datasets/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ def __init__(self):
)
)

add_vote_type(VOTE_REDSHIFT_DATASET_NAME, DatasetIndexer)
add_vote_type(VOTE_REDSHIFT_DATASET_NAME, DatasetIndexer, GET_REDSHIFT_DATASET)

EnvironmentResourceManager.register(RedshiftDatasetEnvironmentResource())
EnvironmentResourceManager.register(RedshiftConnectionEnvironmentResource())
Expand Down
2 changes: 1 addition & 1 deletion backend/dataall/modules/s3_datasets/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def __init__(self):
)
)

add_vote_type('dataset', DatasetIndexer)
add_vote_type('dataset', DatasetIndexer, GET_DATASET)

TargetType('dataset', GET_DATASET, UPDATE_DATASET, MANAGE_DATASETS)

Expand Down
20 changes: 15 additions & 5 deletions backend/dataall/modules/vote/services/vote_service.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,13 @@
from dataall.base.context import get_context
from dataall.modules.catalog.indexers.base_indexer import BaseIndexer
from dataall.modules.vote.db.vote_repositories import VoteRepository
from dataall.core.permissions.services.resource_policy_service import ResourcePolicyService

_VOTE_TYPES: Dict[str, Type[BaseIndexer]] = {}
_VOTE_TYPES: Dict[str, Dict[Type[BaseIndexer], str]] = {}


def add_vote_type(target_type: str, indexer: Type[BaseIndexer]):
_VOTE_TYPES[target_type] = indexer
def add_vote_type(target_type: str, indexer: Type[BaseIndexer], permission: str):
_VOTE_TYPES[target_type] = {'indexer': indexer, 'permission': permission}


def _session():
Expand All @@ -26,9 +27,18 @@ class VoteService:

@staticmethod
def upvote(targetUri: str, targetType: str, upvote: bool):
with _session() as session:
context = get_context()
target_type = _VOTE_TYPES[targetType]
with context.db_engine.scoped_session() as session:
ResourcePolicyService.check_user_resource_permission(
session=session,
username=context.username,
groups=context.groups,
resource_uri=targetUri,
permission_name=target_type.get('permission'),
)
vote = VoteRepository.upvote(session=session, targetUri=targetUri, targetType=targetType, upvote=upvote)
_VOTE_TYPES[vote.targetType].upsert(session, vote.targetUri)
target_type.get('indexer').upsert(session, vote.targetUri)
return vote

@staticmethod
Expand Down
6 changes: 4 additions & 2 deletions frontend/src/design/components/UpVoteButton.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ import * as PropTypes from 'prop-types';
import React from 'react';

export const UpVoteButton = (props) => {
const { upVoted, onClick, upVotes } = props;
const { upVoted, onClick, upVotes, disabled } = props;
return (
<Button
color="primary"
disabled={disabled}
startIcon={
upVoted ? (
<ThumbUpAlt fontSize="small" />
Expand All @@ -27,5 +28,6 @@ export const UpVoteButton = (props) => {
UpVoteButton.propTypes = {
upVoted: PropTypes.bool,
onClick: PropTypes.func,
upVotes: PropTypes.any
upVotes: PropTypes.any,
disabled: PropTypes.bool
};
1 change: 1 addition & 0 deletions frontend/src/modules/Dashboards/views/DashboardView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ const DashboardView = () => {
<Box sx={{ m: -1 }}>
<UpVoteButton
upVoted={isUpVoted}
disabled={!isAdmin}
onClick={() => upVoteDashboard(dashboard.dashboardUri)}
upVotes={upVotes || 0}
/>
Expand Down
11 changes: 6 additions & 5 deletions frontend/src/modules/Redshift_Datasets/views/RSDatasetView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -226,13 +226,14 @@ const RSDatasetView = () => {
</Grid>
<Grid item>
<Box sx={{ m: -1 }}>
<UpVoteButton
upVoted={isUpVoted}
disabled={!isAdmin}
onClick={() => upVoteDataset(dataset.datasetUri)}
upVotes={upVotes}
/>
{isAdmin && (
<span>
<UpVoteButton
upVoted={isUpVoted}
onClick={() => upVoteDataset(dataset.datasetUri)}
upVotes={upVotes}
/>
<Button
color="primary"
startIcon={<ForumOutlined fontSize="small" />}
Expand Down
11 changes: 6 additions & 5 deletions frontend/src/modules/S3_Datasets/views/DatasetView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,13 +266,14 @@ const DatasetView = () => {

<Grid item>
<Box sx={{ m: -1 }}>
<UpVoteButton
disabled={!isAdmin}
upVoted={isUpVoted}
onClick={() => upVoteDataset(dataset.datasetUri)}
upVotes={upVotes}
/>
Comment on lines +270 to +274
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does UpVote take in a disabled prop / does this really do anything?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just shows the number of votes, it allows users to read the upVotes but not to upvote

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure I was a user who had share access to the dataset and was still able to execute upVote - only being stopped on the server side

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, thank you!
image

{isAdmin && (
<span>
<UpVoteButton
upVoted={isUpVoted}
onClick={() => upVoteDataset(dataset.datasetUri)}
upVotes={upVotes}
/>
<Button
color="primary"
startIcon={<ForumOutlined fontSize="small" />}
Expand Down
Loading