Have you tested the modules view which featured are shown only?

I tested and fixed the code. thx @tonypartridge

ummmhh
i guess that featured_up ,featured_down should be added to the #__content_frontpage table

you have deleted the fof/table/table.php file instead of reverting the changes

@alikon the #__content_frontpage table is id sorting, it doesn't control featured status. With the current database structure, the #__content table should be correct.

from my data model POV is wrong, these fields are attributes of the featured and not for the content entity

@eshiol

• does this handle the case where featured_up is now and featured === 0?
  • what is the expected behavior in that case?
• does this handle the case where featured_down is now and featured === 1?
  • what is the expected behavior in that case?

@alikon I don't disagree. It may hit even a point where featured should be in its own table. I was just commenting on the current Joomla core structure which uses the #__content table to control featured items.

@eshiol Please use the PR template for your PR. The template gives you places for the details needed for adding a feature, for how testing should proceed and more.

Pull Request for Issue #13596.

Summary of Changes

Testing Instructions

Expected result

Actual result

Documentation Changes Required

Pull Request for Issue #13596.

Summary of Changes

Added two fields in the article editor featured_up (Start featured) and featured_down (End featured)
An article is featured if featured is yes and (featured_up is null or is less than or equals to now) and (featured_down is null or is greater than or equal to now)

Testing Instructions

create 5 articles:

1. not featured
2. featured, featured_up = null, featured_down = null
3. featured, featured_up = tomorrow, featured_down = null
4. featured, featured_up = null, featured_down = yesterday
5. featured, featured_up = yesterday, featured_down = tomorrow

Create an Articles » Featured Articles menu item
Create a News Flash module and set the option Featured Articles to Only show Featured Articles.

Expected result

Only the articles 2 and 5 will be shown in the Articles » Featured Articles menu item and in the News Flash module.

Actual result

Documentation Changes Required

@eshiol can you please adapt your first Comment with your Comment above? So Tester can read what and how to test and don't have to scroll down.

What about adding featured dates in the "toggle featured status" tooltip?

I agree with @brianteeman until a fix is done MySQL 5.7 this is correct.

On 5 Jul 2017, 19:30 +0100, Brian Teeman notifications@github.com, wrote:

@brianteeman commented on this pull request.
In administrator/components/com_admin/sql/updates/mysql/3.7.3-2017-07-01.sql:

@@ -0,0 +1,2 @@
+ALTER TABLE #__content ADD COLUMN featured_up datetime NOT NULL DEFAULT '0000-00-00 00:00:00';
that is true it has issues (outside of Joomla) BUT we are still using it everywhere so it should stay as proposed for this PR
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@eshiol you will need to adjust the unit test for JTableContentTest with the specific test testCheck
so it doesn't throw Undefined property: JTableContent::$featured_down (I believe you just need to add your new table fields to the test expectations)

Thanks @eshiol I think this PR is now in a testable state. I'm not sure when I will get to testing, I hope within the next week.

I think we need to add something like the published state (Published and is Current; Published, but has Expired; Published, but is Pending) to the featured state (Featured; Featured, but Stopped; Featured, but not yet Started). I would use the icon "expired" for the stopped status, and icon "pending" for the not yet started status). What do you think?

Thats a good idea, I’d go simpler with:

Featured
Featured End: 2017-07-09 10:19:07
Featured Ended:  2017-07-09 10:19:07

On 10 Jul 2017, 09:29 +0100, Helios Ciancio notifications@github.com, wrote:

I think we need to add something like the published state (Published and is Current; Published, but has Expired; Published, but is Pending) to the featured state (Featured; Featured, but Stopped; Featured, but not yet Started). I would use the icon "expired" for the stopped status, and icon "pending" for the not yet started status). What do you think?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

Can you look at resolving the conflicts so that this can be tested please.

@eshiol it looks like you have many not needed files included here?

I merged the remote-tracking branch 'refs/remotes/joomla/3.8-dev', my mistake. But now, I'm not able to revert it.

@eshiol if git revert 956a9cd won't allow you to revert you will likely have to figure out how to redo the source work. (there is probably some way to create a new branch and cherry pick commit 09f5cd5 then overwrite the broken branch in this PR, but I'm not up on all of the pr "rescue" techniques for fixing mistakes and broken branches.)

Just do a git reset --hard upstream/staging if you want to make the branch
the same as the staging branch

--
Tony Partridge

From: Walt Sorensen notifications@github.com notifications@github.com
Reply: joomla/joomla-cms
reply@reply.github.com
reply@reply.github.com
Date: 22 August 2017 at 04:31:36
To: joomla/joomla-cms joomla-cms@noreply.github.com
joomla-cms@noreply.github.com
CC: Tony Partridge admin@xtech.im admin@xtech.im, Mention
mention@noreply.github.com mention@noreply.github.com
Subject: Re: [joomla/joomla-cms] New Feature: Schedule featured articles
#13596 (#16960)

@eshiol https://github.com/eshiol if git revert 956a9cd won't allow you

to revert you will likely have to figure out how to redo the source work.
(there is probably some way to create a new branch and cherry pick commit
09f5cd5
09f5cd5
then overwrite the broken branch in this PR, but I'm not up on all of the
pr "rescue" techniques for fixing mistakes and broken branches.)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#16960 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABVgloes6B-Mu9YvlwJDT9WeXs0136zzks5sakuXgaJpZM4ONEhM
.

Broken PR. I'll open a new one. Sorry.

I just installed 3.8.0 and this feature is not yet available in the backend.

@mijalis As stated above this was closed by the submitter and is awaiting a new pull request