france-ioi / algoreabackend Goto Github PK
View Code? Open in Web Editor NEWBackend for the new Algorea platform
License: MIT License
Backend for the new Algorea platform
License: MIT License
Remove schema_revision
and synchro_version
tables.
The way partial access is propagated from an item to his child is defined is items_items.bAlwaysVisible
and items_items.bAccessRestricted
. In addition to be not very clear, they allow expressing 4 "cases" while there are actually 3 cases.
So, convert items_items.bAlwaysVisible
and items_items.bAccessRestricted
to one single attribute items_items.partial_access_propagation
of type ENUM('none','as grey','as partial')
.
Map the existing data:
Now the new permissions are implemented, use a more appropriate permission rule on who can set additional time.
In contestAdminList and ContestManagedByUser
function (so contestGetGroupByName, contestListMembersAdditionalTime, contestSetAdditionalTime), implement that the one who can administrate a contest ....
? that the one who can modify the additional time are those who can distribute it.
? can watch on the group by which the user has access to the contest
Currently, feature files repeat for each scenario the seed to the db, make it definable once for the whole scenario
In a nutshell, lockUserDeletionDate
is supposed to be used to prevent a user from deleting his account to participate several time to a contest.
This support should be fully implemented by putting it both in the user deletion (already done) but also to prevent a user to leave these groups.
Probably this attribute should be renamed to a more appropriate naming (e.g., user_membership_locked_until
)
In order to do hierarchy checkings for items fast we need to add an index in 'items_items' table on (idItemParent, idItemChild) or on (idItemParent, idItemChild, iChildOrder)
Update the existing contestSetAdditionalTime
service.
For all groups in the contest's items.contest_entering_group_id
, update their group membership expiration to the new contest_started_at + contest_duration + total additional_time
Quick task: Could you add a comment for the tables groups_contest_items
and contest_participations
you have added recently?
From Go 1.11, Go supports semver for dependencies. See https://github.com/golang/go/wiki/Modules
To do:
Everywhere in the code we have error messages starting with capital letters. At the same time, the “Go Code Review Comments” article recommends start them with lowercase letters (https://github.com/golang/go/wiki/CodeReviewComments#error-strings).
To keep HTTP errors starting from uppercase letters we should modify code
return &ErrorResponse{
Response: response,
ErrorText: e.Error.Error(), // FIXME: should be disabled in prod
}
in service.httpResponse() to uppercase the first letter. After doing this we can follow the rule without modifying tests.
I tried to apply our migrations on the loaded prod db on MySQL8 and it fails on some aspect of the schema and data (which is probably invalid in MySQL8).
I had issues on migrations n°2, n°14 (for which I had to do UPDATE history_groups SET sDateCreated = NULL WHERE sDateCreated < '1000-01-01'
), n°17, ...
Edit:
a) The new prod export (from 22/09) should be emptied and considered as the the new "base schema"
b) Then, we have to see what migrations fails with this schema (fixing error of type (1), see above)
c) Then, we have to see what may fail in prod by relaunching the migration and prepend a migration fixing data (fixing error of type (2), see above)
Make sure to do (a), (b), and (c) in this order and in different commits.
As seen in the StructuredLogger middleware, the three following middlewares must be loaded in this specific order:
Otherwise, they cannot do what they are supposed to do. It would be also the opportunity to test this part of app.go
.
#!/bin/bash -eo pipefail
make lint
curl -L https://git.io/vp6lP | sh
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
100 9857 100 9857 0 0 99k 0 --:--:-- --:--:-- --:--:-- 99k
alecthomas/gometalinter info checking GitHub for latest tag
alecthomas/gometalinter info found version: 2.0.12 for v2.0.12/linux/amd64
install: cannot create directory ‘/go:’: Permission denied
Makefile:51: recipe for target 'bin/gometalinter' failed
make: *** [bin/gometalinter] Error 1
Exited with code 2
Contest management will be handled through expiration on group memberships (cfr contest explanation).
So let's first handle expiration without contest.
Let's consider the existing services do not need to set or get this expiration, but membership is only considered as actual if the expiration is not past.
Maybe to make this cleaner in regards of the existing statuses, it would be interesting to invent a new "contestEntered" (or just "system"?) membership, which would be the only one with this expiration for the moment.
Currently, when run make test
it executes BDD tests twice which makes testing slow. It would be much better to execute them just once.
contest_participants_group_id
group for each contestcheckSubmissionRightsForTimeLimitedContest
/getActiveContestInfoForUser
should use the membership expiration (so close*Contest
fcts do not make sense anymore)When an item is created with a duration, create the participant group (with a new type "ContestParticipants", not allowed as input type from these service) as contest_participants_group_id
.
Give this group the partial access on the item.
On edit:
contest_participants_group_id
is NULL, do the same.Update the comment of items.contest_participants_group_id to "Group to which all the contest participants (users or teams) belong. Must not be null for a contest."
In the new definition of the info level (previously grayed), a user with "info" access on an item can see the description, subtitle of the item.
itemView
has to be adapted to reflect this change.
The end goal will be to implement the group permissions as described in https://france-ioi.github.io/algorea-devdoc/design/access-rights/groups/ (and especially at first, the db schema on the top)
Step 2: Introduce group pending requests
Description:
The new "group pending requests" table will be used to store the requests which requires the action from a user (group owner/manager or member). It will allows to get rid of all membership states corresponding to yet-to-be-approved and so non-actual memberships in the groups_groups
table.
Subtasks:
group_pending_requests
table with attributes:
group_pending_requests
groups_groups.type
and groups_groups. type_changed_at
group_pending_requests
in "currentFullUserDataExport" serviceRename the ‘descendants_and_solution_view_propagation’ column to match naming in from the doc.
contest_participations .entered_at
to the attempts table (as contest_entered_at
) as it is related to attempt and not to the group in general.contest_participations
table as it is empty nowHi,
The field groups.sPassword
can be NULL to indicate that a group cannot be joined by password. However, AlgoreaBackend doesn't allow to set this field back to NULL once a password has been set ; the endpoint /groups/{group_id}/change_password
only allows for a new password to be generated.
Would it be possible to add a way to set this field to NULL? (maybe through the /groups/{group_id}/change_password
endpoint)
Thanks!
The table groups_items_propagate
has been created to put away of groups_items
the attribute sPropagateAccess
, in particular to avoid changing groups_items
too often, which run a trigger and store more entries.
However sPropagateAccess
is still stored in both groups_items
and groups_items_propagate
and still groups_items.sPropagateAccess
is still used in the propagation algorithm while it should not.
Remove the usage of groups_items.sPropagateAccess
in the "compute all access" propagation (without breaking it 😁) and remove the attribute from groups_items
.
see specs
Initially only users_items
were used. Then groups_attempts
were added to both implement participation in teams and allow several attempts for the same task.
We now we want to go towards handling a user the same way as a team; and all participations as attempts (even if most items would only allow one attempt at first).
So, we'll merge most of users_items
data into groups_attempts
. In practice, we will keep the users_items
rows (as some of the data is still relevant for now) but the following columns will disappear (they are all in groups_attempts
already):
score
score_computed
score_reeval
score_diff_manual
score_diff_comment
submissions_attempts
tasks_tried
tasks_solved
children_validated
validated
finished
key_obtained
tasks_with_help
hints_requested
hints_cached
corrections_read
precision
autonomy
started_at
validated_at
finished_at
latest_activity_at
thread_started_at
best_answer_at
latest_answer_at
latest_hint_at
contest_started_at
ranked
all_lang_prog
(actually but the ids, state/answer and platform_date_removed)
For the data migration:
users_items
without active_attempt_id
, they have worked as a single user (no team) and have no groups_attempt
for themselves on this item. For these, create a groups_attempts
with the same data + the group_id being the user's self group + order '1'.users_items
with an active_attempt_id
, typically it is a copy of the data should already be in groups_attempts
(there are scores not matching but that's ok to loose them); so we can remove the attributes in users_items without doing anything.The users_items.platform_date_removed
attribute can be removed as well, it has not been used yet, and we'll find a more appropriate solution later.
The ancestors_computation_state
used for propagation of the scores does not make sense anymore.
Finally, the "state" and "answer" attribute are just a duplicate from what is in users_anwers
and can be deleted as well.
Implement new permission rules and propagation described in https://france-ioi.github.io/algorea-devdoc/design/access-rights/items/
Various remarks:
latest_update_on
.content_view_propagation
to the partial propagation, view_propagation_limit
to solution
, grant_view_propagation to true, watch_propagation to true, edit_propagation to truegroupItemEdit
needs major updates. In addition to implement the major rules, it also have to take a new "group_id" input : group as which the user is doing the change.items.bCustomChapter = true
items.bCustomChapter
can be removedWe agreed that these history tables are both hard to maintain and a potential performance issues (for migrations and mass update/delete). We will achieve the same objective with better event logs, sql query logs (provided by RDS) and point-in-time restoration (provided by RDS).
So let's remove all history_*
tables, their triggers and the version
attributes on all tables.
The end goal will be to implement the group permissions as described in https://france-ioi.github.io/algorea-devdoc/design/access-rights/groups/ (and especially at first, the db schema on the top)
Step 1: Introduce group_membership_changes
Description:
The new "group membership changes" table will be used to store the history of changes to memberships, i.e., group-group relationship between any group and a member (only users for now, maybe teams later). It will allows to get rid of all membership states corresponding to former memberships and better track the former states of a membership (not only the last one).
Subtasks:
group_membership_changes
with attributes:
group_membership_changes
(you can put the user himself as initiator for actions initiated by the user, inviting_user_id
for invitation creation)groups_groups
entries with the 'invitationRefused', 'requestRefused', 'removed' and 'left' typesgroupInvitationReject
: do not check anymore that it was already 'invitationRefused'groupsMembershipHistory
: use the group_membership_changes
table (maybe change type_changed_at
to at
only?)invitationsView
: for the *Refused types "within_weeks", use group_membership_changesgroupRequestsView
: for the *Refused types within "old_rejections_weeks", use group_membership_changesgroups_groups.inviting_user_id
group_membership_changes
in "currentFullUserDataExport" serviceIntegrate Swagger to the API so that an API doc can be run (document it rather than automate the doc generation part)
Add a command to delete all temp users with expired sessions.
Remove the related entries (user self groups and co). See https://github.com/France-ioi/AlgoreaPlatform/blob/master/shared/RemoveUsersClass.php#L76
Remarks:
So remove it from the input of the service.
About the "groups_attempts.minus_score" attribute which was added to make index efficient:
For the moment, only a configuration entry which has been defined in the config yml file (whatever its value) can be defined as an ENV variable. If not in the yml, the env var is not retrieved.
groups.PasswordEnd
is defined as "When will the password expire? (set when it's first used)". So maybe it should not be modifiable via the API? It should be set by the backend when the password is used?
In all tables that we use in Go (so not users_threads
, ...), remove the prefix of attribute names and convert the camel case to snake case.
So:
Some examples:
Do not let the requester set a very huge limit (the limit parameter) as it may affect performance and be a base a DoS attack. Max the given value with a fixed value (5000?).
In app/api/answers/get_answers.go
, the spec does not mention 404 while the service may return 404.
Most "datetime" attributes in the db are called "xyz_date" while it is not just a date.
I think "xyz_datetime" is not appropriate as the goal is not to put the type in it but just to have to have something semantically correct.
IMO, "_time" is correct (even if it is not a mysql "time" as well), as a time semantically may be a precise date.
But in most of the time, the attribute naming should not require any suffix, "partial_access_from" would be more appropriate.
BTW, for the durations, I would prefer having them in decimal seconds instead of using the MySQL time format.
Let me know what you plan to do before starting.
Try to upgrade from MySQL to MySQL 8.0 and run the full migrations + tests.
Expected achievements:
db/
."Root" groups are special groups which are defined in the config and set as the initial group of some new users (may be specific for temp users, or admin, ...). These groups are web-domain specific and so config should define the root groups for each served domain. (and probably reject not configured domains?) (so should probably managed in a middleware)
In app/api/auth/create_temp_user.go:82
, the groupID in which temp user has to be put should be retrieved from the config. (so no retrieval based on sType
, sName
, or `sTextId)
The groups.sType
enum('Root','Class','Team','Club','Friends','Other','UserSelf','UserAdmin','RootSelf','RootAdmin')
has be to changed to enum('Class','Team','Club','Friends','Other','UserSelf','Base')
where (for the migration) all "Root", 'RootSelf','RootAdmin' groups should be changed to "Base".
In table users
:
user_id
self_group_id
to group_id
, place it as first columnusers.group_id
to the groups
tableusers. idUserGodfather
which is obsolete (and not used)user_group_id
(11 cases)Cleaning current data for consistency:
groups_groups.inviting_user_id
to non-existing usersgroups_items.creator_user_id
nullable and set to NULL all values not matching a userOur migrations are kind-of tested by applying them on the CI before launching the tests.
Our down-migrations, while they are always written, are not run at all by the CI, so they may be completely erroneous without us noticing it.
It may be very unpleasant to discover that in prod during a critical moment.
So we should include that in the CI testing process.
Fix the behaviour of access right propagation to the following expected behaviour:
For a group A, and let say 2 items M and N; M being the parent of N.
Let's say A is given partial access to M and N (so the 2 groups_items G,M and G,N has both sPartialAccessDate and sCachedPartialAccessDate to "from 2017-01-01".
If G,M is being removed and set to the future, so let's say both sPartialAccessDate and sCachedPartialAccessDate set to "from 2025-01-01"
Then N should not have access to G anymore, i.e. even if G,N still have sPartialAccessDate to "from 2017-01-01", the sCachedPartialAccessDate (the actual access to N) should be "from 2025-01-01".
The computation of sCachedPartialAccessDate
for an groups_items
should be GREATEST( sCached*AccessDate, MIN(all parents.sCachedPartialAccessDate))
(+ bAccessRestricted rules)
Issue around https://github.com/France-ioi/AlgoreaBackend/blob/master/app/database/group_item_store_compute_all_access.go#L157.
Only for partial access.
Try to categorize properly services in Swagger, try to keep only one tag per service
Add some of the reported missing indices:
users_answers
.idAttempt
to select answers by idAttemptgroups
.sName
for sorting in groupChildrenViewgroups
.iGrade
for sorting in groupChildrenViewgroups
.sType
for sorting in groupChildrenViewA declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.