Publishing Simplified #1212
Publishing Simplified #1212
Conversation
|
@stopfstedt I need some help with this. For some reason the test user has lost permission to POST a cohort. But I can't figure out what I changed to make that happen. |
|
@saschaben this is our first 'big change'. I'm not sure how we want to handle discussion and approval. |
|
☔ The latest upstream changes (presumably #1213) made this pull request unmergeable. Please resolve the merge conflicts. |
|
This should not be merged until after the 3.1 release. |
jrjohnson
force-pushed
the
1041-publishing
branch
from
68030a4
to
85fa350
Compare
|
just reviewing; in the migration, are we planning on leaving the existing but deprecated (and orphaned) |
|
I believe we discussed and decided to just drop it. It's dropped in the migration on line 40. |
|
cool thnks! |
| $publishEvent = $offering->getSession()->getPublishEvent(); | ||
| return isset($publishEvent); | ||
| $session = $offering->getSession(); | ||
| return ($session && $session->isPublished()); |
There was a problem hiding this comment.
filter out offering if owning course isn't published. see #1244.
|
LGTM otherwise. |
|
click-tested publishing workflow thru the frontend, holds up for me. |
|
☔ The latest upstream changes (presumably #1245) made this pull request unmergeable. Please resolve the merge conflicts. |
This was never used and doesn’t make sense to keep.
This sets the published boolean for anything that used to have a publish event and removes the published event concept completely.
Use published boolean instead
If we lock this then we cannot create a new cohort in the cohort test. Permission is denied.
jrjohnson
force-pushed
the
1041-publishing
branch
from
85fa350
to
34426d6
Compare
| 's.title, st.sessionTypeCssClass, ' . | ||
| 's.publishedAsTbd as sessionPublishedAsTbd, s.published as sessionPublished, ' . | ||
| 'c.publishedAsTbd as coursePublishedAsTbd, c.published as coursePublished'; | ||
| $qb->add('select', $what)->from('IliosCoreBundle:School', 'school'); |
There was a problem hiding this comment.
double trouble! please remove.
Rebase introduced a double line of the same code, this fixed that.
| @@ -458,17 +459,16 @@ protected function getIlmSessionEventsFor($id, \DateTime $from, \DateTime $to, a | |||
| { | |||
| $qb = $this->_em->createQueryBuilder(); | |||
| $what = 'ilm.id, ilm.dueDate, ' . | |||
| 's.updatedAt, s.title, s.publishedAsTbd as sessionPublishedAsTbd, st.sessionTypeCssClass,' . | |||
| 'pe.id as publishEventId, cpe.id as coursePublishEventId, c.publishedAsTbd as coursePublishedAsTbd'; | |||
| 's.title, st.sessionTypeCssClass, ' . | |||
There was a problem hiding this comment.
please add s.updatedAt, to string here.
This got removed during the rebase, shouldn't have been
This is now comming out of the API correctly. Yay!
jrjohnson
force-pushed
the
1041-publishing
branch
from
4076166
to
f42dd5e
Compare
| @@ -70,7 +68,7 @@ protected function getData() | |||
| 'cohorts' => ['1'], | |||
| 'reminders' => ['1', '2'], | |||
| 'pendingUserUpdates' => [], | |||
| 'permissions' => ['2', '3', '1'] | |||
| 'permissions' => ['1', '2', '3'] | |||
There was a problem hiding this comment.
should be 'permissions' => ['2', '3', '1']
There was a problem hiding this comment.
This fails on my machine. I wonder why it is different..... :(
There was a problem hiding this comment.
Yup, linux and OSX process this differently. No idea why. Maybe just to annoy me.
There was a problem hiding this comment.
I ended up explicitly sorting user permissions. Seems to work.
For whatever reason this is unreliably sorted which makes tests fail. This sorts suer permissions by the permission ID constantly.
|
@stopfstedt ready for review! |
Instead of publish events we no use a published boolean on courses, session, programs, and program years.
Publishing was completely removed from offerings.
Fixes #1041
Complementary frontend ticket ilios/frontend#1294