search

NationalCentreTruthReconciliation / Secure-Record-Transfer

A transfer portal incorporating CAAIS and BagIt Specifications
https://secure-record-transfer.readthedocs.io/en/latest/
4 stars 2 forks source link

Generate model instances from form_data #105

Closed whikloj closed 2 years ago

whikloj commented 2 years ago

First pass at integrating some of the new Models into the submission handling.

Sorry this languished a bit, I was re-using the existing form handling code. So I added a test so I could understand what it was doing.

More to come

whikloj commented 2 years ago

@danloveg This PR on your branch (which will also need to be rebased against the main branch) is getting quite huge.

The parts that I think are missing is some cleanup of all the various models when the Submission is complete and is then deleted.

But it could also use some eyes and testing. I'm going to make a new branch which is all your work and my work rebased to work against the master branch so I can deploy it.

If you prefer I can also open a new PR of this branch directly against master.

danloveg commented 2 years ago

Thanks for bringing this to my attention.

I'd say we close this pull request as soon as possible to avoid it getting more out of hand. Do you recommend I merge your branch in, test, and merge to master, or do you have some other plan in mind? A new merge request can be opened on this branch after that's done to address the things you see as still missing.

whikloj commented 2 years ago

If you have time to look this over and try it out then I think this can be merged. It does require some work to clean up the Metadata models when a Submission is deleted, but that is relatively low hanging.

danloveg commented 2 years ago

Sure, I do have some time tomorrow, I'll make a point to review this then.

danloveg commented 2 years ago

@whikloj I fought with Docker and the migrations not working all morning, I'll have to take a look again on Monday. Did you ever have issues with migration 0027 not running because there's no accession_identifier column?

whikloj commented 2 years ago

You know I have been running this with a special docker settings file. Let me give this a try again with the normal docker just in case I've missed something that is not throwing errors for me.

whikloj commented 2 years ago

@danloveg sorry I'm realizing I was incorrect. I did have some issues with migrations when I rebased on master, but not the one you are referring to.

If it helps I a branch that is all your work and my work rebased against master. https://github.com/whikloj/NCTR-Bagit-Record-Transfer/tree/issue-78

Give it a try and let me know if it works for you.

whikloj commented 2 years ago

Closing in favour of #108