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

[#12048] SeedDb code for student entities #12990

Closed

Conversation

marquestye
Copy link
Contributor

Part of #12048

Outline of Solution

  • Added method to seed students in SeedDb.java
    • Currently adds only one student per course, since the number of courses and accounts seeded are the same

@NicolasCwy NicolasCwy added s.ToReview The PR is waiting for review(s) c.Task Other non-user-facing works, e.g. refactoring, adding tests labels Apr 6, 2024
Copy link
Contributor

@NicolasCwy NicolasCwy left a comment

Choose a reason for hiding this comment

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

LGTM, after #12986 is merged update this branch and use any shared constants before we merge it

rand.nextInt(3) > 1 ? null : getRandomInstant(), // set deletedAt randomly at 25% chance
false);
ofy().save().entities(course).now();
} catch (Exception e) {
log(e.toString());
}

seedStudents(i, courseId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should seedStudents be in the try block? such that if there is an exception then seedStudents will not run

Copy link
Contributor

Choose a reason for hiding this comment

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

Makes sense, could abstract course creation to a function and put both of the methods in the try block

Copy link
Contributor

@FergusMok FergusMok left a comment

Choose a reason for hiding this comment

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

Great job, LGTM

@marquestye marquestye closed this Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c.Task Other non-user-facing works, e.g. refactoring, adding tests s.ToReview The PR is waiting for review(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants