Skip to content

Conversation

@jvwong
Copy link
Member

@jvwong jvwong commented Sep 28, 2020

  • Available status fields, which hopefully are self-evident
    • INITIATED
    • SUBMITTED
    • PUBLIC
    • TRASHED

Refs #843

- Always use status enums declared in Document model
- Update status getters and setters: carousel, export and document API
@jvwong jvwong requested a review from maxkfranz September 28, 2020 19:13
Copy link
Member

@maxkfranz maxkfranz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Two comments:

(1) Are you planning on converting all the existing documents manually with the new status? Or should this be handled with an automatic conversion in the code?

(2) The naming of isPublic() is a bit inconsistent with the other status methods. I suppose public() and publicize() would line up with your other methods, but this isn't very important.

@jvwong jvwong mentioned this pull request Sep 29, 2020
@jvwong
Copy link
Member Author

jvwong commented Sep 29, 2020

Looks good. Two comments:

(1) Are you planning on converting all the existing documents manually with the new status? Or should this be handled with an automatic conversion in the code?

At this point I can migrate this easily with such a simple change. My approach has been to do it 'offline' (local) then load it into an updated instance

(2) The naming of isPublic() is a bit inconsistent with the other status methods. I suppose public() and publicize() would line up with your other methods, but this isn't very important.

Yeah I just went with the most unambiguous. Didn't feel like public sounded like a getter

@maxkfranz maxkfranz self-requested a review September 30, 2020 16:49
Copy link
Member

@maxkfranz maxkfranz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. Let's just merge it in

@jvwong jvwong merged commit a41ab56 into unstable Sep 30, 2020
@jvwong
Copy link
Member Author

jvwong commented Sep 30, 2020

I'm gonna clear out the db at some point ...

@jvwong jvwong deleted the iss843_doc-status-fields branch September 30, 2020 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants