oknozor / musicbrainz_rs

A wrapper around the musicbrainz API
MIT License
38 stars 18 forks source link

feat(recordings): add missing incs for recordings #49

Closed ritiek closed 3 years ago

ritiek commented 3 years ago

Addresses #7.

ritiek commented 3 years ago

There seems to be something wrong with fetching url relations. For example, if I test it on some recording entity:

extern crate musicbrainz_rs;

use musicbrainz_rs::entity::recording::*;
use musicbrainz_rs::prelude::*;

fn main() {
    let senorita = Recording::fetch()
        .id("62f09fd2-144a-439a-96f9-ce93f05b48ae")
        .with_url_relations()
        .execute()
        .unwrap();

    println!("{:?}", senorita);
}

I get the output as:

Recording {
  id: "62f09fd2-144a-439a-96f9-ce93f05b48ae",
  title: "Señorita",
  video: Some(false),
  length: Some(190838),
  disambiguation: Some(""),
  isrcs: None,
  relations: Some([
    Relation {
      end: None,
      attributes: [],
      content: None,
      attribute_values: {},
      attribute_ids: {},
      target_credit: "",
      source_credit: "",
      ended: false,
      type_id: "7e41ef12-a124-4324-afdb-fdbae687a89c",
      begin: None,
      direction: "forward",
      relation_type: "free streaming"
    },
    Relation {
      end: None,
      attributes: ["video"],
      content: None,
      attribute_values: {},
      attribute_ids: {
        "video": "112054d5-e706-4dd8-99ea-09aabee36cd6"
      },
      target_credit: "",
      source_credit: "",
      ended: false,
      type_id: "7e41ef12-a124-4324-afdb-fdbae687a89c",
      begin: None,
      direction: "forward",
      relation_type: "free streaming"
    }
  ]),
  releases: None,
  artist_credit: None,
  aliases: None,
  tags: None,
  rating: None,
  genres: None,
  annotation: None
}

(which can be compared to what the actual response should be)

Now our output seems to be missing the url field in Relation. Should I simply add the url field to our Relation struct? I am also not sure what the purpose of the impl_includes! macro is here, and if this is related in our case. Any idea on how should I proceed with this?

ritiek commented 3 years ago

Ignore my last comment. Turns out I had to update ResponseContent to expect Url entities. Anyway, this should be ready for review now.

But I'm still not sure what the purpose of the impl_includes! macro here is.