Refactor database abstraction layer in Login

RESOLVED FIXED

Status

RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: kate, Assigned: kate)

Tracking

Details

(Whiteboard: [login][June13])

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
Right now we only have access to a single model. Let's expose the model(s) directly so we have more flexibility
(Assignee)

Comment 1

5 years ago
Created attachment 8434410 [details]
https://github.com/mozilla/login.webmaker.org/pull/274/files

So I took a crack at this, basically what I tried to do in this patch was: 

* refactor /models to be /db, and initialize the DB separately from the model definitions
* expose an object containing User, which includes the controller methods 
* consolidate the API in the user model/user controller files into a single API
Attachment #8434410 - Flags: review?(jon)
(Assignee)

Updated

5 years ago
Blocks: 1011176
Comment on attachment 8434410 [details]
https://github.com/mozilla/login.webmaker.org/pull/274/files

this looks like a pretty good set of changes overall. Some minor things to fix up noted in the PR.

I'm not sure of the timing of this landing vs the new functionality and I were working on. Lets discuss tomorrow, and see how we can reduce the pain
Attachment #8434410 - Flags: review?(jon) → review-
(Assignee)

Updated

5 years ago
Attachment #8434410 - Flags: review- → review?(jon)
Comment on attachment 8434410 [details]
https://github.com/mozilla/login.webmaker.org/pull/274/files

I think it'll be ready to land after this last set of changes. Can you also rebase on master? No conflicts fortunately, but master changed a lot today
Attachment #8434410 - Flags: review?(jon) → review-
(Assignee)

Comment 4

5 years ago
Comment on attachment 8434410 [details]
https://github.com/mozilla/login.webmaker.org/pull/274/files

ok, last few things fixed up
Attachment #8434410 - Flags: review- → review?(jon)
Comment on attachment 8434410 [details]
https://github.com/mozilla/login.webmaker.org/pull/274/files

noice. r+ with one nit noted in the PR
Attachment #8434410 - Flags: review?(jon) → review+
(Assignee)

Updated

5 years ago
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [June13][login] → [login][June13]
You need to log in before you can comment on or make changes to this bug.