Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renaming a model attribute or deleting an attribute and creating a new attribute with the same args of the deleted one #22

Closed
abrl91 opened this issue Jul 4, 2023 · 4 comments · Fixed by #23

Comments

@abrl91
Copy link

abrl91 commented Jul 4, 2023

I am trying to rename an attribute, but instead, it renames my model:

builder
  .model(model.name)
  .blockAttribute(ID_ATTRIBUTE_NAME)
  .then<ModelAttribute>((attr) => {
      attr.name = UNIQUE_ATTRIBUTE_NAME;
   });

I tried another way, which is finding the attribute I want to rename, taking its args and creating from it a new attribute, from the type (name) I want and then delete the attribute that I don't want:

const argsFromIdAttribute = (
        modelIdAttribute.args[0].value as RelationArray
      ).args;

 // change the @@id attribute to @@unique attribute
      builder
        .model(model.name)
        .blockAttribute(UNIQUE_ATTRIBUTE_NAME, argsFromIdAttribute);

// remove the @@id attribute from the model - I know it will not work because I need to remove a block attribute, but the 
     // lib doesn't have it so consider it as a pseudo code
      builder.model(model.name).removeAttribute(ID_ATTRIBUTE_NAME);

In this case, I am getting the following error: Subject must be a prisma field!

What am I doing wrong? Does this library support what I am trying to do?

@MrLeebo
Copy link
Owner

MrLeebo commented Jul 5, 2023

Hi @abrl91,

You're right, the API isn't very clear about this. The .then() function is always going to return the current subject of the builder.

Subjects can only be nodes that can contain other nodes, such as models which can contain fields or attributes, or fields which can contain attributes. An attribute never becomes the current subject because it doesn't contain anything else. However, I agree that it isn't very satisfying from a user perspective for the builder to make this distinction. It would be nice for the TypeScript system to automatically assign the type of the current subject to .then() so that you would at least know what type you were receiving, but I couldn't figure out how to make that work.

These are the kinds of kinks that I want to make sure are worked out of the interface before releasing v1 of the library, and I'm open to suggestions for how it can be improved.

.removeAttribute() similarly, is expecting to target field attributes, not model attributes. Although I don't think there's any technical reason why it couldn't support both. That would be a good enhancement.

In the meantime, I think this would achieve the effect you're seeking:

    builder.model(model.name).then<Model>((model) => {
      for (const prop of model.properties) {
        if (prop.type === 'attribute' && prop.name === ID_ATTRIBUTE_NAME) {
          prop.name = UNIQUE_ATTRIBUTE_NAME;
          return;
        }
      }
    });

@abrl91
Copy link
Author

abrl91 commented Jul 5, 2023

@MrLeebo Thank you, it works!

@abrl91
Copy link
Author

abrl91 commented Jul 5, 2023

Hi @abrl91,

You're right, the API isn't very clear about this. The .then() function is always going to return the current subject of the builder.

Subjects can only be nodes that can contain other nodes, such as models which can contain fields or attributes, or fields which can contain attributes. An attribute never becomes the current subject because it doesn't contain anything else. However, I agree that it isn't very satisfying from a user perspective for the builder to make this distinction. It would be nice for the TypeScript system to automatically assign the type of the current subject to .then() so that you would at least know what type you were receiving, but I couldn't figure out how to make that work.

These are the kinds of kinks that I want to make sure are worked out of the interface before releasing v1 of the library, and I'm open to suggestions for how it can be improved.

.removeAttribute() similarly, is expecting to target field attributes, not model attributes. Although I don't think there's any technical reason why it couldn't support both. That would be a good enhancement.

In the meantime, I think this would achieve the effect you're seeking:

    builder.model(model.name).then<Model>((model) => {
      for (const prop of model.properties) {
        if (prop.type === 'attribute' && prop.name === ID_ATTRIBUTE_NAME) {
          prop.name = UNIQUE_ATTRIBUTE_NAME;
          return;
        }
      }
    });

The issue with the .then() was confusing because it seems like the subject can get Block or Property

image

About removeAttribute() - I know it is only for a field attribute. I just wrote it as a pseudo code to emphasize what I am trying to do.

BTW, we use your library for a very important feature (in Amplication), and I must say that it really helps us and it is very easy to use and understand.

@MrLeebo
Copy link
Owner

MrLeebo commented Jul 5, 2023

The issue with the .then() was confusing because it seems like the subject can get Block or Property

Yeah, that type definition is too broad. It should be schema.Block | schema.Field

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants