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

Add shares to poll schema #3513

Merged
merged 27 commits into from
May 25, 2024
Merged

Add shares to poll schema #3513

merged 27 commits into from
May 25, 2024

Conversation

dartcafe
Copy link
Collaborator

@dartcafe dartcafe commented May 16, 2024

What has changed with this PR?

The PR addresses a few aspects.

  1. Performance.
    Loading polls was rather slow with a big amount of polls, since there were a lot of database requests, mainly for permission checks and when dealing with the acl.
    The new approach loads all necessary informations for the permission checks from the database via subqueries and joins. This way, permissions can be checked directly inside the poll.
    As a result the old usage of the Acl has been changed and the new Acl carries not any poll informations, but the user information based on the UserSession and the app permissions.
  2. Move permission checks to the service layer, so the controllers are free from the permission checks.
  3. Further steps to prepare creating polls without the necessity to be logged in.
  4. Preparation of the upcomming vue3 migration
    In the result the structure of the API has changed for a better segmentation. The most important change is, that all changable properties are collected inside the configuration node of the json structure.

The following vue3 migration will be done in two steps:
1. Migrating from vuex to pinia and typescript
2. The final migration to vue3 with the options API including updating the stores
3. Further rolling by-the-way migration of the options API to the composition API

@Bagoult FYI, since you are working with the API.

The structure layout is rather final, but may still get some updates in detail. Also the user objects will still receive some optimizations. I plan to get this API (v1.0) finalized.

To dos:

  • Remove loading app settings from publicy accessing users
  • Fix relevant listing
  • Update user objects

Signed-off-by: dartcafe <[email protected]>
Signed-off-by: dartcafe <[email protected]>
Signed-off-by: dartcafe <[email protected]>
Signed-off-by: dartcafe <[email protected]>
Signed-off-by: dartcafe <[email protected]>
Signed-off-by: dartcafe <[email protected]>
Signed-off-by: dartcafe <[email protected]>
@dartcafe
Copy link
Collaborator Author

dartcafe commented May 16, 2024

Unfortunately there is no dbal method to get an abtraction but the syntax is different. Therefore the platforms have to be identified and individual seletcs are necessary.

@ChristophWurst Is there anyone who can verify this on sqlite, postgre and oracle?

The setup should be a poll with some groupshares.
the excpected column should be group_shares= 'share1,share2'

@dartcafe
Copy link
Collaborator Author

@come-nc @artonge The update was bigger than thought. I fear it will too error prone to backport that to the stable-6 branch.

@come-nc what do you think, would it be valuable to think about rebuilding the compatibility back until NC26/NC27 based on the master branch?

@come-nc
Copy link
Contributor

come-nc commented May 21, 2024

@come-nc what do you think, would it be valuable to think about rebuilding the compatibility back until NC26/NC27 based on the master branch?

A colleague is planning to look into that. It may make sense given #3503 is getting too complicated.
This is hoping the work is not more complicated than what was done to make stable-6 compatible with NC26.

@dartcafe
Copy link
Collaborator Author

A colleague is planning to look into that. It may make sense given #3503 is getting too complicated. This is hoping the work is not more complicated than what was done to make stable-6 compatible with NC26.

Yes. @artonge already told me. I think it will be done with replacing the attributes with annotations or just add them and by removing the few incompatibilities you already found.

BTW: This PR will get some more updates, mainly for the js part.

@dartcafe
Copy link
Collaborator Author

@artonge merging, thisd PR is huge enough. Any further fixes will be done in individual PRs.
New beta will be baked later this day

@dartcafe dartcafe merged commit d703e37 into master May 25, 2024
15 checks passed
@delete-merged-branch delete-merged-branch bot deleted the enh/change-poll branch May 25, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants