search

ymcatwincities / openy

The Open Y platform. See README.md below
https://openy.org
GNU General Public License v3.0
49 stars 112 forks source link

PEF: following the content changes #1330

Open AndreyMaximov opened 6 years ago

AndreyMaximov commented 6 years ago

First of, I would love to say big thanks to the team that has been working on PEF. You guys do a great job!

Meanwhile, it seems that some very important functionality is lost and the framework cannot be used out of the box. The thing is none of the updates of the data the new schedules are built upon are taken into account. E.g. if you update a class, it won't get updated in the schedules. If you rename a category (activity content type) the filter on the schedule page will update, but there won't be any classes matching that category.

That's so due to the fact the repeat_schedule module relies on the openy_session_instance implementations of them hooks that were not designed to work with the denormalized structure of PEF table. These hooks just ignore some of the changes as they are not important for session_instance ideology.

I think PEF must implement its own hooks, because: 1) PEF is so much better than session_instance 2) There are no hooks at all if there is no session_instance module enabled (which is not a dependency of PEF) 2) It seems openy_session_instance thing is going to be deprecated soon (see item 1)

I see the word "framework" is being used here, but the vanilla Open Y is not going to work with PEF unless the default correct implementations of entity insert/update/delete hooks are here.

Thanks, Andrey

ddrozdik commented 6 years ago

cc @ygerasimov , @podarok , @YMCA-GTC

YMCA-GTC commented 6 years ago

Thank you @AndreyMaximov . Would you and @Sanchiz recommend that 5J take this on with part of Open Y 2.0, or that I assign it to IXM instead to resolve?

alexschedrov commented 6 years ago

@YMCA-GTC we'll work on it as a part of Open Y 2.0. cc @ddrozdik @AndreyMaximov

YMCA-GTC commented 6 years ago

Thanks 5J team!

podarok commented 5 years ago

@Sanchiz any updates on this task?

alexschedrov commented 5 years ago

@AndreyMaximov @hamrant please give a status update on this and what has been included into the Open Y 2.0 release.

hamrant commented 5 years ago

@Sanchiz @podarok I only know about these 2

See https://github.com/ymcatwincities/openy/pull/1486

Anyway, they are clearly not enough, for example, hook_alter can be implemented here:

podarok commented 5 years ago

Nice, thank you @hamrant

I see that we need a user story here and a correct tagging @AndreyMaximov @Sanchiz could you write a user story for me and @YMCA-GTC @ymcagtc @paige-kiecker to be able to include in Trello? Thank you in advance

AndreyMaximov commented 5 years ago

@podarok @YMCA-GTC @ymcagtc @paige-kiecker There is a PR, that was merged, https://github.com/ymcatwincities/openy/pull/1391. It doesn't fully resolve the issue but makes a big step forward in this direction.

There is also a comment that explains why we couldn't finish the work and what we need to finish it https://github.com/ymcatwincities/openy/pull/1391#issuecomment-450485039

Sorry @hamrant but it seems your comment is about something else. The extendable PEF is a very important topic though, I'll take care of the user story.

AndreyMaximov commented 5 years ago

P.S. sorry again @hamrant :-) Your comment and the mentioned changes relate to the issue indirectly.

paige-kiecker commented 5 years ago

Thanks for the follow up @AndreyMaximov. If you could get that user story to @podarok and me this week, we will get it on our roadmap.

podarok commented 5 years ago

Any news @AndreyMaximov ?

sarah-halby commented 3 years ago

@AndreyMaximov & @Sanchiz - it doesn't look like the user story was provided for this item. We're working on our backlog refinement, and I'd like to get this added. Can you please provide the user story if this issue is still relevant?