strapi / rfcs

RFCs for Strapi future changes
68 stars 33 forks source link

Add Virtual Attribute Type RFC #17

Open Tomaszal opened 3 years ago

Tomaszal commented 3 years ago

Slightly modified my approach from PR #7221 and created this RFC as requested.

RFC Preview Link

remidej commented 3 years ago

I like this idea. Virtual types seem like a great way to make data more readable in the content manager.

Here are some other use cases where it would be useful:

I'm also curious about how it would fit with the Content Types Builder. We probably won't ask the user to write a function in the CTB, but it still has to know that this field exists.

Tomaszal commented 3 years ago

Yeah those are some nice use cases, should I add them to the RFC?

Regarding how it would fit in the Content type Builder - I think it should be kept pretty simple. Only ask for a field name with no advanced settings. Perhaps also add a link to the documentation explaining what it is / how to use it.

alexandrebodin commented 3 years ago

Referencing an article that was initiating prior the the RFC: https://github.com/strapi/strapi/pull/6433

Tomaszal commented 3 years ago

@alexandrebodin any updates on this?

Tomaszal commented 3 years ago

In general I would like you give more details about the content manager (can this be displayed in the list view ? how do we handle sorting, filtering ...) Do we display this field in the edit view ?

@alexandrebodin I don't really see a reason why this couldn't be displayed, sorted or filtered in the list view. I think it should be handled as regular text in there. Perhaps if the perceived 'type' of the virtual attribute for the content manager is a concern there could be an additional field virtualType. That way whenever content manager sees a virtual attribute it will handle it as it would handle an attribute of virtualType. However, I'm not sure how necessary is this (I haven't looked too deep into the content manager, so you probably know better).

If a setter is added to the virtual attribute functions then yes, this could definitely be displayed in the edit view. However then there is a problem with user being confused if both the original values and the virtual one are displayed. I.e. if user fills in first name and last name, and then fills in the full name, one thing or the other will be overridden.

darron1217 commented 3 years ago

How about adding getter and setter on model’s js file?

For example on api/student/models/student.js

module.exports = {
  virtualAttributes: {
    fullName: {
      // getter
      get(model) {
        return `${model.firstName} ${model.lastName}`
      },
      // setter
      set(model, data) {
        const [firstName, lastName] = data.split(' ')
        model.firstName = firstName
        model.lastName = lastName
      },
      // For filter
      filter(options) {
        if(options.fullName) {
          const [firstName, lastName] = options.fullName.split(' ')
          options.firstName = firstName
          options.lastName = lastName
          delete options.fullName
        }
      },
      // For sort
      sort(options) {
        if(options.fullName) {
          const direction = options.fullName
          options.firstName = direction
          options.lastName = direction
          delete options.fullName
        }
      }
    }
  }
};
darron1217 commented 3 years ago

How about implement it as custom field type? Then we can create CombinedText type and define getter, setter, filter handler... etc on custom area. And then just select field type on Admin Page.

brettwillis commented 3 years ago

@Tomaszal here are some of my comments/inputs:

Use-case: value generated in write lifecycle

Consider a system that sends push notifications to users when an item is published, and it sets a notifiedAt: Date attribute with the timestamp of the notification (which is stored in the database). The notifiedAt attribute should be visible to the user so they can see if/when a notification was sent, but they can never edit it (i.e. it is "virtual").

This amplifies the need for async operations during calulation (i.e. sending a push notification).

An alternative/simple way of implementing this is just hard-coding such an attribute as readonly in the model's JSON, but it could be integrated into this proposal if it makes sense?

strapi-cla commented 3 years ago

CLA assistant check
All committers have signed the CLA.

SvenWesterlaken commented 1 year ago

Any updates on this?

Inaztm commented 1 year ago

Any updates on this?

Tomaszal commented 1 year ago

Sorry, I have not used Strapi for any projects since writing this RFC, so I do not have the relevant context to update it with the suggestions from the team. I would suggest someone to take over and update the RFC so that the Strapi team can look into it again.

SvenWesterlaken commented 1 year ago

No problem @Tomaszal. I was mainly addressing the straps team in order to bring attention to this again as it would ease up a lot of data I would like to add to my models.

alexandrebodin commented 1 year ago

No problem @Tomaszal. I was mainly addressing the straps team in order to bring attention to this again as it would ease up a lot of data I would like to add to my models.

Hi, this feature does look interesting but isn't something on the roadmap for the time being. Feel free to upvote and comment your specific usecase here https://feedback.strapi.io/feature-requests/p/field-type-computed :)