Conversation
… requiredInWizard, wizardStep)
…st email on mail step
… setting descriptions to info help icon
dennis-zyska
left a comment
dennis-zyska
left a comment
There was a problem hiding this comment.
Nice implementation, I added a few comments, let me know if anything is unclear
| type: Sequelize.BOOLEAN, | ||
| defaultValue: false, | ||
| }); | ||
| await queryInterface.addColumn('setting', 'wizardStep', { |
There was a problem hiding this comment.
can't that be a FK to the id of the wizard_step table?
| type: Sequelize.STRING, | ||
| allowNull: true, | ||
| }, | ||
| type: { |
There was a problem hiding this comment.
Can we use integer instead of strings in that table? I guess the types are predefined somehow?
There was a problem hiding this comment.
quick verification question: are they still visible for everyone after that?
There was a problem hiding this comment.
Can we move those settings just to our settings table instead of creating a new table just for those two entries?
|
|
||
| module.exports = { | ||
| async up(queryInterface, Sequelize) { | ||
| await queryInterface.addColumn('setting', 'displayName', { |
There was a problem hiding this comment.
can we move those column changes to backend/db/migrations/20260119185850-extend-setting-wizard.js, that would be more readable, the entries can be still in here but we should change the name of the migration into -basic-setting-displayName
| * First-time setup wizard: Admin, General (optional Moodle + JSON import), Mail, Registration, Summary. | ||
| * Fetches /setup/config for steps and wizardSettings. On Finish, calls settingSave and redirects. | ||
| * | ||
| * @author CARE |
There was a problem hiding this comment.
CARE is not the author
| registration: ["Enable registration", "Information requested at registration", "Consent options", "Email verification rate limit"], | ||
| }; | ||
|
|
||
| function wizardValueToFormString(v) { |
There was a problem hiding this comment.
we usually not defining variables outside of the export default, we therefore have the assets, if the function are used many times in the software, otherwise it should be under methods
| return; | ||
| } | ||
|
|
||
| if (!this.$socket.connected) { |
There was a problem hiding this comment.
I'm not sure if we should additional add here the socket connection, maybe we can handle that in the /setup/state route as well and use our default mechanism for the socket connection. Then we don't need it here, also reducing this huge code here a bit
| router.beforeEach(async (to, from, next) => { | ||
| if (to.path === "/wizard") { | ||
| try { | ||
| const r = await fetch(getServerURL() + "/auth/check", { credentials: "include" }); |
There was a problem hiding this comment.
I have the feeling this is not a good idea, first we have two beforeEach checks here, why not integrating it in the other, second, why not checking it inside of the component?
| } | ||
| if (!to.meta.requireAuth && !to.meta.checkLogin) return next(); | ||
| try { | ||
| const r = await fetch(getServerURL() + "/auth/check", { credentials: "include" }); |
There was a problem hiding this comment.
especilly this seems like a lot of requests for each route change
2 participants
Main Description
This branch delivers a guided onboarding experience for CARE by introducing a first-time setup wizard and aligning the admin Settings UI to the same structure and language.
It addresses the original problem that setup was fragmented, technical, and hard to follow by having a clear step flow (admin creation, general, mail, registration, moodle, summary).
#99
New User Features
New Dev Features
Improvements