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

EZP-29729: As an editor, I want to have access to all my drafts #684

Merged
merged 4 commits into from
Oct 26, 2018
Merged

EZP-29729: As an editor, I want to have access to all my drafts #684

merged 4 commits into from
Oct 26, 2018

Conversation

adamwojs
Copy link
Member

@adamwojs adamwojs commented Oct 15, 2018

Question Answer
Tickets https://jira.ez.no/browse/EZP-29729
Bug fix? no
New feature? yes
BC breaks? no
Tests pass? yes
Doc needed? yes
License GPL-2.0

The only place when as an editor I can go back go to the editing of new content item draft is a "Dashboard" > "Me" > "Drafts". Unfortunately, this is not possible when I have more than 10 drafts at once.

This PR solves this issue by adding "My drafts" list in the user context menu (similar to the legacy administration panel).

screenshot_2018-10-20 drafts - ez platform
screenshot_2018-10-20 drafts - ez platform 1
screenshot_2018-10-20 ez platform

Checklist:

  • Coding standards ($ composer fix-cs)
  • Create JIRA issue
  • Align with InVision Prototype
  • Ready for Code Review

@adamwojs adamwojs self-assigned this Oct 15, 2018
@andrerom
Copy link
Contributor

Nice one @adamwojs, will there be link from dashboard to this as well when there is more then 10 drafts?

@lserwatka
Copy link
Member

Yes, this is the idea.

@adamwojs adamwojs changed the title [RFC] As an editor, I want to have access to all my drafts EZP-29729: As an editor, I want to have access to all my drafts Oct 15, 2018

{% block content %}
<div class="row align-items-stretch ez-main-row">
<section class="container mt-4">
Copy link
Contributor

@inakijv inakijv Oct 15, 2018

Choose a reason for hiding this comment

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

Hi, Could you please add offset px-5 for consistency with the rest of views with tables

@inakijv
Copy link
Contributor

inakijv commented Oct 15, 2018

Hi @adamwojs this would be the icon for drafts view.
drafts.svg.zip

DatasetFactory $datasetFactory,
FormFactory $formFactory,
SubmitHandler $submitHandler,
int $defaultPaginationLimit)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
int $defaultPaginationLimit)
int $defaultPaginationLimit
) {

new ArrayAdapter($contentDraftsDataset->getContentDrafts())
);
$pagination->setMaxPerPage($this->defaultPaginationLimit);
$pagination->setCurrentPage(min(max($currentPage, 1), $pagination->getNbPages()));
Copy link
Contributor

Choose a reason for hiding this comment

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

If we care that the user entered ex. -1 as page number, maybe we should inform him about the wrong page number?

@@ -88,6 +89,7 @@ public function mapConfig(array &$scopeSettings, $currentScope, ContextualizerIn
'content_policy_limit',
'notification_limit',
'user_settings_limit',
'my_drafts_limit',
Copy link
Contributor

Choose a reason for hiding this comment

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

we have content_draft_limit and my_drafts_limit. We should operate with one name, like in above.

</thead>
<tbody>
{% if pager.count is same as(0) %}
<tr>
Copy link
Contributor

Choose a reason for hiding this comment

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

missing indentation

</td>
</tr>
{% else %}
{% for row in pager.currentPageResults %}
Copy link
Contributor

Choose a reason for hiding this comment

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

missing indentation


{% if pager.haveToPaginate %}
<div class="row justify-content-center align-items-center mb-2 mt-2 ez-pagination__spacing">
<span class="ez-pagination__text">
Copy link
Contributor

Choose a reason for hiding this comment

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

CS

);

return [
'id' => implode(':', [
Copy link
Contributor

@mikadamczyk mikadamczyk Oct 22, 2018

Choose a reason for hiding this comment

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

this could be a Value Object, maybe in lib/UI/Value/Content?

@adamwojs
Copy link
Member Author

PR updated according to @mikadamczyk suggestions.

@lserwatka
Copy link
Member

@adamwojs can we proceed with QA here?

@barbaragr
Copy link

  1. Please change the message after clicking trash button for: Are you sure you want to permanently delete the selected draft(s)?

  2. Version conflict window isn't displayed
    a. Create CI and make about 3 drafts after that (v2, v3, v4)
    b. Go to User menu/Drafts
    c. Click third version (v3) of the CI and publish it
    d. Go back to the Drafts list and click on edit button next to second version (v2)
    Actual result:
    Console shows: GET http://eev2/admin/version/has-no-conflict/67/3/eng-GB 409 (Conflict) Uncaught (in promise) TypeError: Cannot set property 'href' of null
    Expected result: Version conflict window should appear I suppose.

Copy link

@barbaragr barbaragr left a comment

Choose a reason for hiding this comment

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

  1. If user doesn't have Content/Versionread policy Drafts tab on Dashboard is hidden. But on user menu Drafts are still displayed.

@adamwojs
Copy link
Member Author

PR updated according to @barbaragr suggestions 😉

Copy link

@barbaragr barbaragr left a comment

Choose a reason for hiding this comment

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

Great :)

@lserwatka lserwatka merged commit 9ea409a into ezsystems:master Oct 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

8 participants