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

[ticket/16193] Re-implement drafts #5759

Open
wants to merge 5 commits into
base: 3.3.x
Choose a base branch
from

Conversation

v12mike
Copy link
Contributor

@v12mike v12mike commented Nov 30, 2019

rebased to 3.3.x

PHPBB3-16193

Checklist:

  • Correct branch: master for new features; 3.3.x & 3.2.x for fixes
  • Tests pass
  • Code follows coding guidelines: master, 3.3.x and 3.2.x
  • Commit follows commit message format

Tracker ticket (set the ticket ID to your ticket ID):

https://tracker.phpbb.com/browse/PHPBB3-16193

v12mike added 3 commits November 30, 2019 14:56
rebased to 3.3.x

PHPBB3-16193
Fix indentation

PHPBB3-16193
Fix indentation

PHPBB3-16193
@marc1706
Copy link
Member

marc1706 commented Dec 6, 2019

I have assigned this the 4.0 label as I don't think we'll be able to merge this refactoring into 3.3 due to the rather short time we have before its RC and stable release.

@v12mike
Copy link
Contributor Author

v12mike commented Dec 6, 2019

I have assigned this the 4.0 label as I don't think we'll be able to merge this refactoring into 3.3 due to the rather short time we have before its RC and stable release.

Does "4.0" mean 4.0, or something after 3.3.0?

@koraldon
Copy link

koraldon commented Dec 6, 2019

Will be great if it can happen for 3.3 😇 seems like a small but very good feature

@SiteSplat
Copy link

@koraldon
Copy link

koraldon commented May 3, 2020

Seeing that font awesome was merged into 3.3, any chance to see this as well?

@ghost
Copy link

ghost commented May 3, 2020

I'll see if I can review this aswell.
As it stands though, this PR has conflicts that need to be resolved.
If you need assistance with resolving conflicts, please let us know.

@v12mike
Copy link
Contributor Author

v12mike commented May 3, 2020

I should be able to fix the conflicts in a day or two...

This PR is currently against the 3.3.x branch.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

This too is a rather large, large PR, that will require a lot of testing on local environments too.
A lot of new functionalities and changes to existing ones..
Not a big fan of how drafts will be handled, where they take presedence over a lot of other common actions, eg: if ($visibility == ITEM_DRAFT) { ... } else { // Everything else, like a regular post }. Not to mention the amount of additional nested conditionals.

@@ -1138,7 +1009,7 @@ function topic_review($topic_id, $forum_id, $mode = 'topic_review', $cur_post_id
'S_HAS_ATTACHMENTS' => (!empty($attachments[$row['post_id']])) ? true : false,
'S_FRIEND' => ($row['friend']) ? true : false,
'S_IGNORE_POST' => ($row['foe']) ? true : false,
'L_IGNORE_POST' => ($row['foe']) ? sprintf($user->lang['POST_BY_FOE'], get_username_string('full', $poster_id, $row['username'], $row['user_colour'], $row['post_username']), "<a href=\"{$u_show_post}\" onclick=\"phpbb.toggleDisplay('{$post_anchor}', 1); return false;\">", '</a>') : '',
'L_IGNORE_POST' => ($row['foe']) ? sprintf($user->lang['POST_BY_FOE'], get_username_string('full', $poster_id, $row['username'], $row['user_colour'], $row['post_username']), "<a href=\"{$u_show_post}\" onclick=\"phpbb.toggleDisplay('{$post_anchor}', 1); return false;\">", '</a>') : '',
Copy link

Choose a reason for hiding this comment

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

Why are these changed to tabs?

@@ -1564,7 +1445,7 @@ function submit_post($mode, $subject, $username, $topic_type, &$poll_ary, &$data
}
else if ($mode == 'edit')
{
$post_mode = ($data_ary['topic_posts_approved'] + $data_ary['topic_posts_unapproved'] + $data_ary['topic_posts_softdeleted'] == 1) ? 'edit_topic' : (($data_ary['topic_first_post_id'] == $data_ary['post_id']) ? 'edit_first_post' : (($data_ary['topic_last_post_id'] == $data_ary['post_id']) ? 'edit_last_post' : 'edit'));
$post_mode = ($data_ary['topic_posts_approved'] + $data_ary['topic_posts_unapproved'] + $data_ary['topic_posts_softdeleted'] == 1) ? 'edit_topic' : (($data_ary['topic_first_post_id'] == $data_ary['post_id']) ? 'edit_first_post' : (($data_ary['topic_last_post_id'] == $data_ary['post_id']) ? 'edit_last_post' : 'edit'));
Copy link

Choose a reason for hiding this comment

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

Why are these changed to tabs?


$data_ary['topic_visibility'] = $topic_row['topic_visibility'];
$data_ary['post_visibility'] = $topic_row['post_visibility'];
$post_visibility = ITEM_DRAFT;
Copy link

Choose a reason for hiding this comment

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

What's the point of this variable as it is immediately overwritten by ITEM_APPROVED?

@@ -1624,6 +1514,7 @@ function submit_post($mode, $subject, $username, $topic_type, &$poll_ary, &$data
{
$post_visibility = (in_array((int) $data_ary['force_visibility'], array(ITEM_APPROVED, ITEM_UNAPPROVED, ITEM_DELETED, ITEM_REAPPROVE))) ? (int) $data_ary['force_visibility'] : $post_visibility;
}
}
Copy link

Choose a reason for hiding this comment

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

Everything above is not properly indented.

// Delete draft if post was loaded...
$draft_id = $request->variable('draft_loaded', 0);
if ($draft_id)
if ($data_ary['post_visibility'] != ITEM_DRAFT)
Copy link

Choose a reason for hiding this comment

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

Missing an emtpy line

@@ -494,6 +496,11 @@
$user->setup('posting');
trigger_error('NO_POST');
}
if ($mode == 'soft_delete' && $post_data['post_visibility'] == ITEM_DRAFT)
Copy link

Choose a reason for hiding this comment

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

New line before an if block.

// Egosearch and draftsearch are author searches
case 'draftsearch':
$target_visibility = ITEM_DRAFT;
// deliberate drop-through
Copy link

Choose a reason for hiding this comment

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

// no break is used throughout

@@ -1094,7 +1104,7 @@
$topic_unapproved = (($row['topic_visibility'] == ITEM_UNAPPROVED || $row['topic_visibility'] == ITEM_REAPPROVE) && $auth->acl_get('m_approve', $forum_id)) ? true : false;
$posts_unapproved = ($row['topic_visibility'] == ITEM_APPROVED && $row['topic_posts_unapproved'] && $auth->acl_get('m_approve', $forum_id)) ? true : false;
$topic_deleted = $row['topic_visibility'] == ITEM_DELETED;
$u_mcp_queue = ($topic_unapproved || $posts_unapproved) ? append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=queue&amp;mode=' . (($topic_unapproved) ? 'approve_details' : 'unapproved_posts') . "&amp;t=$result_topic_id", true, $user->session_id) : '';
$u_mcp_queue = ($topic_unapproved || $posts_unapproved) ? append_sid("{$phpbb_root_path}mcp.$phpEx", 'i=queue&amp;mode=' . (($topic_unapproved) ? 'approve_details' : 'unapproved_posts') . "&amp;t=$result_topic_id", true, $user->session_id) : '';
Copy link

Choose a reason for hiding this comment

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

Why is this changed?

@@ -1,6 +1,4 @@
<fieldset class="fields1">
<!-- IF not S_SHOW_DRAFTS -->
Copy link

Choose a reason for hiding this comment

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

This entire file now has incorrect indentations.

@@ -186,30 +196,48 @@ <h2 class="searchresults-title"><!-- IF SEARCH_TITLE -->{SEARCH_TITLE}<!-- ELSE
<dt class="author">{L_POST_BY_AUTHOR} <!-- EVENT search_results_post_author_username_prepend -->{searchresults.POST_AUTHOR_FULL}<!-- EVENT search_results_post_author_username_append --></dt>
<dd class="search-result-date">{searchresults.POST_DATE}</dd>
<dd>{L_FORUM}{L_COLON} <a href="{searchresults.U_VIEW_FORUM}">{searchresults.FORUM_TITLE}</a></dd>
<!-- IF searchresults.TOPIC_REPLIES >= 0 -->
Copy link

Choose a reason for hiding this comment

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

Wrong indentation and should use twig syntax for newly added template code.
The twig syntax also applies to a lot more files.

v12mike added 2 commits May 5, 2020 04:45
Merge branch '3.3.x' of https://github.com/phpbb/phpbb into ticket/16193

PHPBB3-16193
Merge branch '3.3.x' of https://github.com/phpbb/phpbb into ticket/16193
PHPBB3-16193
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants