Refactor/cms subservice

[JohanHjelsethStorstad]

This is a big PR that uses the new subOperation system to refactor the cms and implement it as part of the different systems that need it.

[Copilot]

Pull Request Overview

This PR refactors the CMS (Content Management System) to use a new subOperation system, implementing CMS functionality as modular sub-operations that can be composed by different services. The changes replace direct CMS function calls with operation-based implementations, introduce ownership checks for resource access control, and consolidate permissions under new categories like FRONTPAGE_ADMIN and PUBLIC_ARTICLE_ADMIN.

Key changes:

• Introduced defineSubOperation and implement patterns for composable CMS operations
• Replaced CMS_ADMIN permission with more granular permissions (FRONTPAGE_ADMIN, PUBLIC_ARTICLE_ADMIN)
• Refactored all CMS services (images, paragraphs, links, articles, articleSections) to use the new operation system
• Added ownership checks to ensure services can only modify resources they own
• Updated database schema field names (e.g., shortname → shortName)

Reviewed Changes

Copilot reviewed 213 out of 227 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
src/services/serviceOperation.ts	Core changes to support sub-operations with ownership checks and implementation fields
src/services/permissions/constants.ts	Updated permission categories, replacing CMS_ADMIN with granular permissions
src/services/cms/*/operations.ts	Converted CMS services to use new operation system with sub-operations
src/services/*/operations.ts	Various services implementing CMS sub-operations with ownership checks
src/prisma/schema/*.prisma	Schema updates for field naming consistency (shortname → shortName)
src/app/**/*.tsx	Updated component usage to pass configured actions with implementation params

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Paulijuz]

Amazing! This one has been a long coming and it's really a big milestone to have this finally done. I feel you came up with one of the cleanest possible solutions given our architecture.

I don't have anyspecific requests for this PR, its large enough as it is, but I have some general thoughts for the future.

I feel like authorizer and ownershipCheck could be one function. I feel like the division is a bit artifical and from what i can see they are just used in conjunction within the subservice implementation anyway. I think I see why it was done this way, but I feel like it the need for this split stems from a weakness of the current implementation of the Auther system rather than any actual logical requirement.

I feel like SubService operation has quite many (too many?) different generic and parameters. The whole implementation feels way more complex than what it actually does. I don't especially like implementationParams, it kinda breaks the whole point of an abstraction in that the highest level shouldn't need to know about how the lower levels work. I see why it would be difficult to implement and if i understand you correctly you gave it a shot to combine both implementaitonParams and params? However i still think we should give it another shot as it would make the interface simpler. I can take a look at it if you want? I don't think it would be to hard to change later as it would just be a merging of implementation params and params, the logic would stay the same.

Not really related to what you have done, but I think it might be beneficial to export baseSchema from schemas.ts. I have experienced situations where its nice to reference some fields of another model and then it makes more logical sense to reference the base rather than one of the derived schemas. This would of course entail that we should have more descriptive names than "base", but code quality wise i think that would also be an improvement.

Another thing is that i never really liked the word "special". With this PR we have an opportunity to change that. I really like the "special" cms system, but the word is not really descriptive of what it does. For example, read one of your comments

Check if a link with id is special with special at[t]ribute in the provided special array.

Wouldn't "specialPurpose", "uniquePurpose", "systemPurpose", just "purpose", "reservedFor" or "systemUseCase" be for example better? Try reading again:

Check if a link has one of the special purposes in the provided array of special purposes.

If you want to keep "special" that's fine, but then we should clearly explain somewhere what the heck a "special" article is. But in my opinion, most people would find it easier to understand that an article has a special/unique/system purpose rather than it being special and having a special attribute.

Again, just food for thought and none of this needs to be handeled in this PR. I thinks its better to just merge it as is.

Finally, I just say that its always nice to see comments here and there. Especially for such complicated systems. <3

LGTM!

[Paulijuz]

Beklager for en vegg av tekst, men det var en lang PR så føler det var fair :P

[JohanHjelsethStorstad]

I Get that special is not descriptive. Actually static might be more descriptive. It is hard because, well, the special parts are used for many different things. One thing I however think is that with this new system a service at least has to take explicit ownership over parts of the special cms (giving it more explicit purpose in the process). I agree that the implementationparams are sort of bad but you need a way to identify the resource using the subresource so to say. I like to think of it as chaining. When implementing a service you first specify the params and data used in the inner sublayer then the implementationparams used in the outer layer (note that the implementationparams are only actually accessible in the .implement) in the definition of a subservice you can naturally know nothing of them (so in this sense there is separation of concerns, specifically params)

I disagree that ownershipCheck and authorizer should be merged. If they were a user could call for example the updateCommitteeArticle action and be told that they are unauthorized even though the problem is that the user is trying to update a news article through the wrong action (assuming the user is authorized to update the specific article). I think the distinction handels two completely different concerns 1. does the user have ownership to the service 2. Does the service own the subservice. However I think the name ownershipCheck could be misleading. Note that I purposely made the check for ownership not have access to session. Because it should have nothing to do with the session. (Also I removed access to the session in the getter of the authorizer as it should only be fed in to the authorizer. Anything else would be strange)

[JohanHjelsethStorstad]

Also "ownershipCheck" is probably not the hottest name. It might leave the impression that "does the user (session) own the resource". Perhaps a more long name like "resourceOwnsSubresourseCheck" however this is verbose

[Paulijuz]

Ah yes, I see now, my bad. From skimming the code i really thought that ownershipCheck was that the user owned the given resource. Then I agree that they should stay separate, but also that it definitively needs another name.

[Paulijuz]

It is hard because, well, the special parts are used for many different things.

Would you say that those many different things are many different purposes?

[JohanHjelsethStorstad]

Well yes each thing implementing special is a purpose but each purpose consists of an array of different special resources. For example a (special) cmsImage could be purposed for the front page service. However many cms images share this purpose so as I see this we cannot call the field purpose

[JohanHjelsethStorstad]

owned

Yeah it proves it needs a better name...

[Paulijuz]

I agree that the implementationparams are sort of bad but you need a way to identify the resource using the subresource so to say.

I agree that the implementation needs to be able to get information. I just find it clunky that from the caller perspective i have to differentiate whether the argument goes to the implementation of the subservice or the subservice itself. When you call a method on a derived class in c++ or python or whatever you don't group the arguments based on whether they go to the base class or derived class. I think the same thing should apply here.

[Paulijuz]

However many cms images share this purpose...

I guess this depends on if you view being part of the frontpage as one single purpose or many different ones (frontpage 1, frontpage 2, frontpage 3, etc... being different purposes). I personally find the latter way of thinking natuarl, but I can see why some won't. Anyways my point is just that i find the name confusing and I think we both can agree that there must exist a better alternative

[Paulijuz]

We should probably stop discussing here as this PR is closed, we can continue the discussion on discord or wherever if you want