search

WikiEducationFoundation / WikiEduDashboard

Wiki Education Foundation's Wikipedia course dashboard system
https://dashboard.wikiedu.org
MIT License
385 stars 600 forks source link

Add Redirect For Requests For The Current Term Of Alerts #5805

Closed cyrillefr closed 3 weeks ago

cyrillefr commented 1 month ago

What this PR does

This PR aims to close #5800

With a new route rule to go to the new current_alerts method in the Campaign controller. Inspired by the alerts method from same controller.

The show method had to be modified because React calls some campaign/slug.json after populating with some other calls. Whether in html or json format. So the current has to be interpolated to the real current Campaign Current is the same as default

Open questions and concerns

I believe one could add also this bookmark: http://domain/campaigns/current/programs quite easily

ragesoss commented 1 month ago

What if we try to make a campaign with the slug current? I guess that should be prevented with a validation in campaign.rb, or some other way.

cyrillefr commented 1 month ago

Hello @ragesoss ,

I need to check about the validation. In case it is possible, this record should be updated with the values of the current campaign. Or may be some kind of link if it is possible. I will investigate and in case post some code.

cyrillefr commented 1 month ago

Hello @ragesoss ,

One can dup a Campaign object and give it the current slug (and also title). But such an object has no courses. There is a need to do some other works to get the copy to link to associated objects of the current Campaign.

But also, how to keep these 2 objects in synch ?

We would need to add a hook on save/update on Campaign, check if it is about the current campaign, and if so, proceed to update current.slug Campaign. In my opinion, this is a bit heavy.

ragesoss commented 1 month ago

@cyrillefr i was thinking that we should just use a validation on the Campaign model to make sure we never have an actual campaign with the slug current. Then way you've implemented it in the controller will work fine.

cyrillefr commented 1 month ago

Aaaah :) I added the validation.

cyrillefr commented 1 month ago

Checks failed at linting stage. But for a source file version that is not part of my branch .... File spec/controllers/articles_controller_spec.rb that have a line too long. Version of my branch is older and does not have this modification. A bit strange

ragesoss commented 1 month ago

My fault. I added that linting violation while fixing an urgent but last week. Don't worry about that failure.