Differential D12188

Save and delete a user
Needs ReviewPublic
Actions

Authored by rishabhg on Apr 14 2018, 11:06 AM.

Details

Reviewers
jjazeix
timotheegiet
Summary

To be merged in Branch Network Client

  • Give an option to save and delete a user
  • Create a user only if name ,DOB and password fields are not empty

Diff Detail

Repository
R2 GCompris
Lint
Lint Skipped
Unit
Unit Tests Skipped
rishabhg created this revision.Apr 14 2018, 11:06 AM
Restricted Application added a project: KDE Edu. · View Herald TranscriptApr 14 2018, 11:06 AM
Restricted Application added a subscriber: KDE Edu. · View Herald Transcript
rishabhg requested review of this revision.Apr 14 2018, 11:06 AM
jjazeix added a comment.Apr 14 2018, 1:43 PM

I can't run the fix:
qrc:/gcompris/src/server/views/UsersManagement.qml:302:9: Type AddUpdateUser unavailable
qrc:/gcompris/src/server/views/AddUpdateUser.qml:163:30: Expected token `,'

src/server/views/AddUpdateUser.qml
44

string

48

coding convention

101

passwordColumn

147

GCText

150

qsTr()

222

the aim was to be able to add multiple users at same time, not just one

rishabhg added a comment.Apr 15 2018, 7:45 AM
In D12188#246058, @jjazeix wrote:

I can't run the fix:
qrc:/gcompris/src/server/views/UsersManagement.qml:302:9: Type AddUpdateUser unavailable
qrc:/gcompris/src/server/views/AddUpdateUser.qml:163:30: Expected token `,'

Oh! this is because the 'Delete' button can't have the id as 'delete'. I guess, it conflicts with the built in type
This has been fixed in the last commit though.
Will fix it in this commit now

rishabhg added inline comments.Apr 15 2018, 2:18 PM
src/server/views/AddUpdateUser.qml
101

can you elaborate more ?
change 'pc' to passwordColumn ?

222

Yes, there should be a functionality that allows to save all the users at the same time.
how about a 'saveAll' button ?

rishabhg updated this revision to Diff 32202.Apr 15 2018, 5:38 PM
  • corrected coding covention
  • used qsTr() and GCText wherever required
  • implemented delete functionality
NOTE: The first user created does not get saved until the focus from 'Date of birth' column is explicitly changed. Could not figure the reason for this
rishabhg marked 5 inline comments as done.Apr 15 2018, 5:39 PM
jjazeix added inline comments.Apr 16 2018, 6:23 AM
src/server/views/AddUpdateUser.qml
46

they are all strings?

54

empy log?

101

yes the aim is to understand the variable when we read them on the code. pc will mean nothing for a new person that will read the code where passwordColumn will tell him that it corresponds to the password column of the table

282

why do you need an empty user?
If it is for: "NOTE: The first user created does not get saved until the focus from 'Date of birth' column is explicitly changed. Could not figure the reason for this", we should find the reason and fix it (force the focus change?)

rishabhg added inline comments.Apr 18 2018, 4:12 PM
src/server/views/AddUpdateUser.qml
46

right, they are.
Missed it some how

54

ah! mistake

282

why do we need an empty user ?

We need to have something in our model so that our delegates get displayed. Maybe a better solution exists
I'll try something on my end.

If it is for: "NOTE: The first user created does not get saved until the focus from 'Date of birth' column is explicitly changed. Could not figure the reason for this",

No. Also, just found out that this is the case for every user that's created.

we should find the reason and fix it (force the focus change?)

forcing the focus change solves the problem.

rishabhg updated this revision to Diff 32684.Apr 21 2018, 6:02 AM
  • changed type of dob and password to String
  • removed empty logs
  • force the focus on text field after clicking on 'save' button
rishabhg marked 4 inline comments as done.Apr 21 2018, 6:04 AM

Revision Contents
Changeset List

Diff 32684

View Options

src/server/views/AddUpdateUser.qml

Loading...
View Options

src/server/views/UsersManagement.qml

Loading...