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.

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

Ks 2021 12 token voting permissions #4106

Merged
merged 7 commits into from
Dec 23, 2021
Merged

Conversation

Rineee
Copy link
Contributor

@Rineee Rineee commented Dec 21, 2021

I did some more research and decided to not use rules for the token voting api permissions as rules are dependent on users and arent really useful here.
I instead introduced a new Permission class, called TokenVotePermission that checks manually the relevant requirements on the token for create and destroy actions.
Also, I added a check in TokenVoteMixin to make sure that the token is valid before anything else is checked.

@Rineee
Copy link
Contributor Author

Rineee commented Dec 21, 2021

@fuzzylogic2000 Now I remember why I also wanted to add the phase to the serializer, we also use the isVotingPhase for not showing the ratings here:


Not sure how to deal with it if not with serializer info, do you have an idea?

@fuzzylogic2000 fuzzylogic2000 added the Dev: A4 depending PR or issue dependent on A4 label Dec 22, 2021
@fuzzylogic2000
Copy link
Contributor

@fuzzylogic2000 Now I remember why I also wanted to add the phase to the serializer, we also use the isVotingPhase for not showing the ratings here:

Not sure how to deal with it if not with serializer info, do you have an idea?

So, the decision was to just keep it in the template tag for now, right?

@Rineee
Copy link
Contributor Author

Rineee commented Dec 22, 2021

@fuzzylogic2000 Now I remember why I also wanted to add the phase to the serializer, we also use the isVotingPhase for not showing the ratings here:

Not sure how to deal with it if not with serializer info, do you have an idea?

So, the decision was to just keep it in the template tag for now, right?

Yes! Should I write an issue to think about moving it to the api?

@Rineee Rineee changed the title WIP Ks 2021 12 token voting permissions Ks 2021 12 token voting permissions Dec 23, 2021
@Rineee Rineee force-pushed the ks-2021-12-token-voting-permissions branch from 75d97e9 to ce36b85 Compare December 23, 2021 13:24
@Rineee Rineee force-pushed the ks-2021-12-token-voting-permissions branch from 6e91d5f to d764239 Compare December 23, 2021 13:47
@github-actions
Copy link

github-actions bot commented Dec 23, 2021

Coverage report

Total coverage

Status Category Percentage Covered / Total
🔴 Statements 11.75% 157/1336
🔴 Branches 9.82% 75/764
🔴 Functions 11.94% 53/444
🔴 Lines 18.03% 711/3943

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Show files with reduced coverage 🔻

Reduced coverage

Status Filename Statements Branches Functions Lines
🔴 meinberlin/apps/budgeting/assets/VoteButton.jsx 0% (-33.33% 🔻) 0% (-25% 🔻) 0% (-16.67% 🔻) 0% (-69.7% 🔻)
🟢 meinberlin/apps/budgeting/assets/BudgetingProposalListItem.jsx 100% 50% (-30% 🔻) 100% 100%

Status of coverage: 🟢 - ok, 🟡 - slightly more than threshold, 🔴 - under the threshold

Report generated by 🧪jest coverage report action from 1267280

@Rineee Rineee force-pushed the ks-2021-12-token-voting-permissions branch from d764239 to 1267280 Compare December 23, 2021 14:55
@Rineee Rineee requested a review from sabinammm December 23, 2021 14:56
Copy link
Contributor

@philli-m philli-m left a comment

Choose a reason for hiding this comment

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

install and passing tests but vote button functioonality no longer working, is this expected?

@Rineee
Copy link
Contributor Author

Rineee commented Dec 23, 2021

install and passing tests but vote button functioonality no longer working, is this expected?

@phillimorland No, that should work still! What does it say in the network tab?

@philli-m
Copy link
Contributor

@Rineee ignore me, I forgot I had to re-enter my token :p however I will add that we must disable hover styling on the disabled vote button

@philli-m philli-m merged commit 492dbeb into main Dec 23, 2021
@philli-m philli-m deleted the ks-2021-12-token-voting-permissions branch December 23, 2021 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dev: A4 depending PR or issue dependent on A4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants