andrewbayly
commented
5 months ago David
Thanks for your feedback and for the opportunity to re-visit my work! This is the first time I’ve come across this approach, and think that more technical interviews could benefit from the ability to respond to feedback.
I’ll resubmit the exercise by the end of the week.
One clarification:
UI: Not much responsible UI has been in place - do you mean “responsive UI” ?
If there is anything else, I’ll surface it in GitHub.
Kind regards
—Andrew
On Mar 25, 2024, at 9:05 PM, David Horak @.***> wrote:
Hi Andrew, Thank you for your submission, below are some notes.
Good
Clear outline of the project in the reame. Testing Despite tests are just manually, it has been helpful to read trough the testing scenarios. Areas of improvements
These are notes upon which you can optionally update the submission. Also, this is not a comprehensive list, rather some high level notes, if you would feel that you can improve the solution by other changes, making the implementation more suitable to expose to real users, please do so.
Accessibility: Could we apply some beset practices for accessibility, e.g. title for actionable elements. UI: Not much responsible UI has been in place. UX: Significant non-reversible actions should be confirmed by users before executing. E.g. delete a user. Missing any sort of notification about completed actions for the users (add, edit, delete) - the data simply change on the screen - if not hidden behind the screen fold - long list. How we would handle a long list of users on the listing page? The profile image is not implemented there to ever present placeholder image does not provide much value. Data model: The member model as field called Admin, which is a boolean field. This may be limiting consider that already support 2 roles, which could be expanded in the future. Input Validation I was able to enter bogus for both email and phone number. System integrity: I was able to delete all users. Let's assume that only admins would be able to add users, how we could setup the app to prevent a dead-end situation? I was able to create members with the same email / phone number, ending up with duplicate records Navigation: not mapped urls give me an a hard 404 server error, e.g.: http://127.0.0.1:8000/bogus Tests: As you outlined the testing has been done manually, it would great to validate some of the functionality via tests Kind regards, David (Instawork)
— Reply to this email directly, view it on GitHub https://github.com/andrewbayly/members/issues/1, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAYH6DQ4OI2KD6ZFYGAKKTY2DXY7AVCNFSM6AAAAABFIFK7CCVHI2DSMVQWIX3LMV43ASLTON2WKOZSGIYDOMJZG4ZTSNY. You are receiving this because you are subscribed to this thread.
davidhorak
Hi Andrew, Thank you for your submission, below are some notes.
Good
Areas of improvements
Admin, which is a boolean field. This may be limiting consider that already support 2 roles, which could be expanded in the future.bogusfor both email and phone number.Kind regards, David (Instawork)