mdelobelle / metadatamenu

For data management enthusiasts : type and manage the metadata of your notes.
https://mdelobelle.github.io/metadatamenu
MIT License
534 stars 28 forks source link

postValues seems to adjust the properties incorrectly #571

Closed TheAtomAnt closed 9 months ago

TheAtomAnt commented 9 months ago

I recently altered my code to take into account the indexedPath need in the FieldsPayload object. I create a few of these objects and then call the MM api postValues.

The fields get updated, but a dash is put in front of each updated field. It's almost as if the field is being updated to be a list item of the previously named field.

Type: Person Age: 10 FavoriteColor: Red

After preforming an update of the age and color, I get the following:

Type: Person -Age: 18 -FavoriteColor: Blue

I didn't notice this issue prior to the addition of the indexedPath property, and the new way of looking up indexedPath values.

Also, how do I look up the indexedPath's properly anyway? I see there is a fileFields api, which when called provides an object filled with FieldInformation, however, the documentation doesn't seem to indicate how I would convert from the field "name" to the field "indexedPath". I do see there is an undocumented name property in the FieldInformation object, which is what I'm using now, but being undocumented, makes me feel I'm doing it wrong.

mdelobelle commented 9 months ago

Hi which version of MDM are you currently using? I've re-written the API in 0.8.0, just want to know from where I should start looking for a fix

If you're running 0.7.7, could you test your use case with 0.8.0-beta-1? https://github.com/mdelobelle/metadatamenu/releases/tag/0.8.0-beta-1

TheAtomAnt commented 9 months ago

I am currently using 0.7.7, I'll give 0.8.0-beta-1 a try. Actually, are there code differences between 0.7.7 and 0.8.0? I fetched the repo and graphed the tags, head/master/0.7.7/0.8.0-beta-1 all seem to be at the same point. there is a "refacto-to-bulk" that is ahead, is that the one that you want me to try?

TheAtomAnt commented 9 months ago

Ok, I pulled the three files using the link you sent - main.js, styles.css, version.json the same issue happen.

I noticed that the - is being put in front of both the normal properties up in the YAML section, and when I update a dataview styled property propertyname::value, inline with the text.

whatever is going on, this appears to be an issue with 0.8.0-beta-1 as well.

Every property that I update has a "-" added to the front, as if it is part of a list.

TheAtomAnt commented 9 months ago

Here is what I'm doing to get the indexedPath, which is a bit off putting, but it's all I could figure out right now.

       var fileFields = await this.mm.fileFields(file.path);
        let indexLookups: any = {};
        for(var key in fileFields){
            if(key=="fileclass-field-Type") continue;
            var f = fileFields[key];
            indexLookups[f.name] = f.indexedPath
        }

I create the batch of updates like this...

       /// NOTE: my implementation of FieldsPayload has a constructor that arranges the value correctly
        var propertyUpdates = [
            new FieldsPayload(indexLookups["TaskComplete"], isTaskComplete.toString()),
        ];

Last I call the update

await this.mm.postValues(file, propertyUpdates, -1, true, false, false);
TheAtomAnt commented 9 months ago

I want to include my implementation of the FieldsPayload and the FieldPayload classes, just in case... I know I'm sort of naming things incorrectly here due to the FieldsPayload being defined as an array of anonymous types.

export class FieldsPayload {
    indexedPath: string;
    payload: FieldPayload;

    constructor(propertyindexedPath: string, propertyValue: string) {
        this.indexedPath = propertyindexedPath;
        this.payload = new FieldPayload(propertyValue);
    }
}

export class FieldPayload {
    value: string;
    constructor(propertyValue: string) {
        this.value = propertyValue;
    }
}
TheAtomAnt commented 9 months ago

Ok. I got it. Perhaps the documentation is incorrect on the field ordering? https://mdelobelle.github.io/metadatamenu/api/#postvalues or, perhaps I stupidly messed up the ordering on my end. Simply removing all the properties from the postValues call other than the file and the updates, fixed the issue.

await this.mm.postValues(file, propertyUpdates)
mdelobelle commented 9 months ago

You're right the documentation is not correct. The "after" argument doesn't exist anymore. I'll fix that for the public release of 0.8.0

TheAtomAnt commented 9 months ago

Thanks. Also, with updating documentation - guidance on how to property convert from property name to the indexedPath would be very welcome.

mdelobelle commented 9 months ago

Sure will do I've introduced a new api method that handles the legacy "postValues" way of updating fields with names instead of indexedPath: postNamedFieldsValues which signature is the same as the legacy postValues (including the after argument) In case you don't have nested object fields, this may also be a way to facilitate your use case?

TheAtomAnt commented 9 months ago

I did see that method :) However, I found it after I updated my code to the new postValues method. I don't mind the new method, but to use it, I would need a way to convert from the field name to the indexedPath. The modification you made is reasonable and will allow editing more advanced properties, like objects, more efficiently.

One idea would be to modify the return object of fileFields, to include properties that reference the field both by the ID, and the name of the property, perhaps with dot notation if possible: example.

Imagine a page that has the following fields:

FileClass: Person
Age: 10
Location:
      - Name:Home
        Address: Somewhere
        IsPrimary: True
      - Name:Work
        Address: Somewhere else
        IsPrimary: False

The name of the properties on the return object could ALSO include the following, along with the existing properties labeled by the ID of the field...

returnObject["FileClass"]
returnObject["Age"]
returnObject["Location[0].Name"]
returnObject["Location[0].Address"]
returnObject["Location[0].IsPrimary"]
returnObject["Location[1].Name"]
returnObject["Location[1].Address"]
returnObject["Location[1].IsPrimary"]

That way I could do this:

    let ageIndexPath = returnObject["Age"].indexPath;
    let ageId =  returnObject["Age"].id;
    assertTrue(  returnObject["Age"] ===  returnObject[ageId]  );

Just some random thoughts.

mdelobelle commented 9 months ago

I could totally do that.

I could expose another api method "namedFileFields" that would return the same Object values but having the "named" indexedpath as keys

instead of a dotted syntax, I would go for "____" to separate the fields since "." can be used as a valid character in yaml keys (at least Obsidian's yaml parser doesn't interpret it as a nested property); and it is very unlikely that someone would use 4 underscores in their fields names 😅

so "Location[0]____Name" instead of "Location[0].Name"

WDYT?

mdelobelle commented 9 months ago

you can test it in the last beta release https://github.com/mdelobelle/metadatamenu/releases/tag/0.8.0-beta-4

TheAtomAnt commented 9 months ago

Hey, Thanks!

"____" works for me!

Also, I tested 0.8.0-beta-4, and the new method works exactly as described and provides the lookup I need to convert a known field name to an indexedPath value.

You cleaned up my code by removing the manual mapping of indexedPath to the name loop.

I noticed the value of the field was not included, which was part of my original musing. In this case, I don't need the old value, so it's not a big deal, but if you intended for the value to be included when calling this new method, it might be a bug.

Thank you for this quick work! Great plugin, and great support!

TheAtomAnt commented 9 months ago

Other than the rest of the documentation work, I think this is well sorted!