Skip to content

multiple drafts support#68

Closed
yasser-sobhy wants to merge 2 commits intocopasetickid:masterfrom
Hsoub:master
Closed

multiple drafts support#68
yasser-sobhy wants to merge 2 commits intocopasetickid:masterfrom
Hsoub:master

Conversation

@yasser-sobhy
Copy link

@yasser-sobhy yasser-sobhy commented May 30, 2017

Hi, I am using draftsman for a quite large project at my company, that project needs a multiple drafts functionality, unfortunately draftsman lakes for that. So I decided to support multiple drafts and push draftsman to version 1.

Changes:

  • remove deprecated methods and callbacks
  • change has_drafts to has_draftsman
  • change draft? to has_drafts?
  • every update is considered a drafts, no updates to existing drafts
  • change belongs_to association to has_many
  • change draft association name to drafts
  • most functions now operates on a ActiveRecord::Associations::CollectionProxy instead of Draftsman::Draft

To change:

  • edit readme.md to reflect new changes
  • review draft_publication_dependencies and draft_reversion_dependencies to make them work with multiple drafts or remove them if possible
  • make sure new changes passes all tests or add tests for new functionality

@chrisdpeters
Copy link
Collaborator

@yasser-sobhy Thank you for the pull request!

I am very interested in this feature, but I think that some things would need to change with the implementation.

  1. Don't make it version 1. I'd probably release this as 0.7 or 0.8 honestly. So put those
    deprecated methods back in too. Focus this pull request just on the new feature instead of
    trying to manage the overall project.
  2. Create an optional setting on .has_drafts that lets the developer decide if they're going to
    have a single draft per record or multiple drafts.
  3. Bring #draft? back as an alias of #has_drafts? for the single draft scenario that I mention
    above.
  4. Change .has_draftsman back to .has_drafts. There is no real conflict there because that
    is a class-level method.

There is probably some opportunity to refactor this logic into includeable modules, kind of the way that concerns work in Rails. Maybe it should be architected so that the single draft and multiple draft strategies are their own modules that mix in common logic and implement the custom bits that each needs.

If you don't have the bandwidth to make these changes, I can probably get around to it eventually. I'll be honest though: I don't need this feature myself, so I don't have much motivation myself (though again, I'd love to see support for it).

Let's discuss. Anyone, jump in.

@yasser-sobhy
Copy link
Author

@chrisdpeters I have cloned the latest Dratfsman with Rails 5.1 support and rewritten the changes again and separated single and multiple drafts functionality as you suggested. Have a look at my PR #71 .

@chrisdpeters
Copy link
Collaborator

Closing this in favor of #71.

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.

2 participants