Meeds-io / MIPs

The Meeds Improvement Proposal repository
0 stars 0 forks source link

Upgrade news data model to note Part 2 #130

Closed margondicco closed 3 months ago

margondicco commented 5 months ago

Rationale

1. Functional Requirements

First part : https://github.com/Meeds-io/MIPs/issues/119#issue-2102504523

Second part :

2. Technical Requirements

Feature Flags

Property Name Default Value Target Audience Functional Behaviour
newsScheduleAndFilterDisplaying false users enable/disable new scheduling and filters in news app except draft filter

The FT flag will be deleted at the end of dev.

Configurability

Content addon

Upgradability

Content addon

3. Software Architecture

Services & processing

Same as MIP-119

Migration strategy

Notes addon

Content addon

azayati commented 5 months ago

Ready for tech review @boubaker (just after Go-func cc @Julien-Dubois-eXo @srenault-meeds )

boubaker commented 4 months ago

@azayati can you please include in the technical requirements the migration to use Spring. All new addon introduced in Meeds has to be based on Spring and no more on deprecated Kernel & WS modules.

srenault-meeds commented 4 months ago

Ok for me.

One thing to check please @margondicco You tell that the flag will be deleted but I still see it listed in the MIP's description

azayati commented 4 months ago

Ok for me.

One thing to check please @margondicco You tell that the flag will be deleted but I still see it listed in the MIP's description

yes @srenault-meeds the FT flag will be deleted at the end of this iteration. I have added that in the description.

srenault-meeds commented 4 months ago

ok

azayati commented 4 months ago

@azayati can you please include in the technical requirements the migration to use Spring. All new addon introduced in Meeds has to be based on Spring and no more on deprecated Kernel & WS modules.

@boubaker I have added that requirement in Upgradability section:

We should springify existing service & rest layer

boubaker commented 4 months ago

@azayati can you please include in the technical requirements the migration to use Spring. All new addon introduced in Meeds has to be based on Spring and no more on deprecated Kernel & WS modules.

@boubaker I have added that requirement in Upgradability section:

We should springify existing service & rest layer

ok, thanks. Ok for me

Julien-Dubois-eXo commented 4 months ago

@srenault-meeds @boubaker FYI we had to enable the ability to create a note or a news with the same title. It was something available for the news so we have to keep it and it was a pain point for the note.

Julien-Dubois-eXo commented 4 months ago

@srenault-meeds @boubaker FYI we had to enable the ability to create a note or a news with the same title. It was something available for the news so we have to keep it and it was a pain point for the note.

@srenault-meeds @boubaker FYI about this point we have decided not to do it right now due to technical complexity. We will just manage during the migration the case of news with the same title. If we want it we need more technical study to do it safely.

FYI @margondicco

srenault-meeds commented 4 months ago

Questions:

Julien-Dubois-eXo commented 4 months ago

@azayati I let you answer as it's a technical decision.

azayati commented 4 months ago
  • would it be possible to use a title of a content previously used but the content has been deleted?

It is possible now without modification.

  • how do you manage news posted in a space and other posted in another space with same title?

It is not possible right now with the limitation of notes.

  • why is that complicated?

The models and services are designed to have the name considered as a key for the note page, we can not retrieve many notes with same name, so this needs a reflexion and decision if we should allow to create many notes having the same title with the same name or with random names (as for drafts). Anyway a lot of changes and tests should be done and this is not the context of that MIP IMO

margondicco commented 4 months ago

Hello agree with @srenault-meeds , the restriction on the title is a big regression (for news). My case : i create each week an article "Weekly reporting". Imagine that some clients will create more than 10 000 notes and article per year with more than 1000 spaces....

My proposal : we will move this requirement in the next MIP

srenault-meeds commented 4 months ago

Ok to move this requirement in the next MIP #128 Let me know as soon as it is added in the fonctional specifications

azayati commented 3 months ago

Ready for Review by DAO Members ( Meeds: @srenault-meeds @boubaker)

PRs and ACCs Up and Running

srenault-meeds commented 3 months ago

LGTM.

Two observations though:

Julien-Dubois-eXo commented 3 months ago

If you have removed the FF, then delete it from the MIP description IMO for better clarity (I have tested it using runtime fetch and it seems to be removed but I wasn't sure)

@srenault-meeds I prefer to keep them. It's part of the specifications. If I remove it, it is we who will lose clarity. The MIP should also be practical for us. I cannot have two different specifications in several places.

srenault-meeds commented 3 months ago

I'm sorry to insist about it but if this is not included anymore in the implementation, this is not clear then to keep it.

boubaker commented 3 months ago

LGTM.

Ok for me as well, Thanks.