Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added page copy action to admin #380

Merged
merged 16 commits into from Aug 1, 2014
Merged

Conversation

kaedroho
Copy link
Contributor

This builds on my previous PR (#379) and adds admin UI for page copy

@davecranwell
Copy link
Contributor

I'm afraid I missed a subtlety here when you emailed around the spec: Wouldn't the user need to be able to choose where to copy the page(s) to? Having to make a copy then move it - two steps - is undesirable.

@kaedroho
Copy link
Contributor Author

I think in most cases, users would just want to copy into the same section of their website. Maybe we could add an optional step after the "set title and slug" form to allow them to move the copied page into a different section of the site at the same time if they wanted to.

@davecranwell
Copy link
Contributor

I think that would be nice yeah. In a way it's the terminology that necessitates choosing a destination: "Duplicate" tends to imply "into the same section" whereas "Copy" is tightly coupled with "Paste" which can usually occur anywhere, generally speaking. I think being able to "copy" without being able to choose your "paste" is pretty counter-intuitive, so even if it's not a common requirement to copy to another section, it should definitely be an option.

I'd suggest you try and reuse the page chooser modal, as we would when choosing a link during page editing, placed below the title/slug fields, like you said.

@sixpearls
Copy link
Contributor

Could the page chooser modal default to the current location (location of the original page)? This assumes the chooser handles going up the tree well, which I haven't played with.

@davecranwell
Copy link
Contributor

I'm sure it could. I haven't tried it myself, but it's just a case of telling it which url to open on launch.

@chrxr chrxr added this to the 0.4 milestone Jun 30, 2014
@chrxr
Copy link
Contributor

chrxr commented Jun 30, 2014

Hi @kaedroho, we want to review and merge this as it stands in 0.4. @davecranwell please create an enhancement ticket for 0.5.

Thanks.

@chrxr chrxr assigned gasman and unassigned kaedroho Jun 30, 2014
@davecranwell
Copy link
Contributor

Done #390

Conflicts:
	wagtail/wagtailadmin/views/pages.py
@gasman
Copy link
Collaborator

gasman commented Jul 10, 2014

I think we need more consideration of how this works with limited-permission users before it's ready to be merged. Some gotchas that spring to mind (may not be an exhaustive list)...

  • Users without 'publish' permission on the destination section should not be able to create pages that are immediately live on the site. (My gut feeling is that all copied pages should be left unpublished after a copy, regardless of the user's permission level.)
  • The copy action should respect the destination section's subpage_types setting, and not allow pages to be copied if that page type is not on the list. (This is only an edge case at the moment, since the copy remains in the same section as the original, but will become more relevant when we allow other destination sections to be selected.)
  • Ownership of the new page(s) should be assigned to the user doing the copying (otherwise users with only 'add' permission will end up creating pages that they can't edit).

@tomdyson tomdyson modified the milestones: 0.5, 0.4 Jul 10, 2014
…to kaedroho-page-copy-ui

Conflicts:
	wagtail/wagtailadmin/tests/test_pages_views.py
…lug validation method there rather than pushing one in from the view
…to CopyForm itself. This also allows us to move the help text into the actual help_text property, which fixes the dodgy spacing on the form field
gasman added a commit that referenced this pull request Aug 1, 2014
Added page copy action to admin
@gasman gasman merged commit 2579bc0 into wagtail:master Aug 1, 2014
@kaedroho kaedroho deleted the page-copy-ui branch September 11, 2015 08:23
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.

None yet

6 participants