chore(API): Include changed_by.id in Get Charts and Get Datasets API responses

[Vitor-Avila]

SUMMARY

The API responses for fetching all charts (/api/v1/chart/) and all datasets (/api/v1/dataset/) don't include the changed_by.id attribute. This is useful when implementing automations to maintain content (that involve reaching out to the user that last modified it).

This PR includes this field in the two API responses. It is already available when fetching all dashboards (/api/v1/dashboard/).

TESTING INSTRUCTIONS

1. Make sure you have at least 1 chart and 1 dashboard created.
2. Overwrite/modify both of them.
3. Send a GET request to /api/v1/chart/ and confirm the changed_by.id is visible there.
4. Send a GET request to /api/v1/dataset/ and confirm the changed_by.id is visible there.

ADDITIONAL INFORMATION

• Has associated issue:
• Required feature flags:
• Changes UI
• Includes DB Migration (follow approval process in SIP-59)
  • Migration is atomic, supports rollback & is backwards-compatible
  • Confirm DB migration upgrade and downgrade tested
  • Runtime estimates and downtime expectations provided
• Introduces new feature or API
• Removes existing feature or API

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c186ea3) 69.16% compared to head (582bd6e) 69.16%.
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #26540      +/-   ##
==========================================
- Coverage   69.16%   69.16%   -0.01%     
==========================================
  Files        1948     1948              
  Lines       76062    76062              
  Branches     8493     8493              
==========================================
- Hits        52610    52609       -1     
- Misses      21272    21273       +1     
  Partials     2180     2180              

Flag	Coverage Δ
hive	53.68% <ø> (ø)
mysql	78.06% <ø> (+0.02%)	⬆️
postgres	78.16% <ø> (ø)
presto	53.63% <ø> (ø)
python	82.86% <ø> (-0.01%)	⬇️
sqlite	77.75% <ø> (ø)
unit	55.89% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Vitor-Avila]

hey @eschutho @betodealmeida @john-bodley while working on this PR, I noticed that the response for getting all charts (/api/v1/chart/) includes more fields than when getting a single chart (/api/v1/chart/<pk>).

Is this by design? I would expect the same amount of fields, or the other way around (fetching a specific chart returns more information about it than fetching all charts).

What are your thoughts on making sure that:

• All asset endpoints (charts, datasets, dashboards, etc) return the same fields (when available)
• Fetching a single resource returns the same information than fetching all resources

I would imagine this wouldn't require a SIP (given we're not removing fields from the API or adding complexity) but would love to get your opinion.

[john-bodley]

@Vitor-Avila regarding your comment:

Is this by design? I would expect the same amount of fields, or the other way around (fetching a specific chart returns more information about it than fetching all charts).

I sense this is simply one (of many) inconsistencies which currently exist in our APIs—likely because they were probably written to handle specific use cases (both from a request and response perspective), i.e., historically the APIs (which evolved from Flask views) were built in a vertical rather than horizontal manner. There are pros/cons with both approaches where one could argue that the current approach is optimized for the UX, i.e., payload reduction where possible.

There likely needs to be a concerted effort to ensure that our APIs are consistent.

[Vitor-Avila]

Thanks for the additional context, @john-bodley!

[sadpandajoe]

🏷️ preset:2024.3