feat: switch from files.url to files.azul_url (#4575) #4581
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request standardizes file download URL handling across the codebase by replacing the url property with azul_url in both Anvil-CMG and HCA-DCP implementations. This change ensures consistent naming conventions for accessing file download links throughout the application.
Key changes:
- Updated entity interface definitions to use
azul_urlinstead ofurl - Modified view model builders to reference the new
azul_urlproperty when constructing file download objects
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/apis/azul/anvil-cmg/common/entities.ts | Updated FileEntity interface to replace url with azul_url property |
| app/apis/azul/hca-dcp/common/entities.ts | Updated FileResponse interface to replace url with azul_url property |
| app/viewModelBuilders/azul/anvil-cmg/common/viewModelBuilders.ts | Modified buildFileDownload function to reference azul_url instead of url |
| app/viewModelBuilders/azul/hca-dcp/common/viewModelBuilders.ts | Modified buildFileDownload function to reference azul_url instead of url |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| */ | ||
| export interface FileResponse { | ||
| accessible: boolean; | ||
| azul_url: string; |
There was a problem hiding this comment.
The new azul_url property is defined as non-nullable string, while the removed url property was string | null. If the API can return null values for this field, this type definition could cause runtime errors. Verify the API response guarantees a non-null value, or update the type to string | null to maintain backward compatibility.
| azul_url: string; | |
| azul_url: string | null; |
| */ | ||
| export interface FileEntity { | ||
| accessible: boolean; | ||
| azul_url: string; |
There was a problem hiding this comment.
The new azul_url property is defined as non-nullable string, while the removed url property was also string. Consider whether this field should allow null values for consistency with the HCA-DCP implementation, especially if the underlying API might not always provide a URL.
| azul_url: string; | |
| azul_url: string | null; |
There was a problem hiding this comment.
@MillenniumFalconMechanic HCA had a TODO comment regarding the url property (whether null was a possible value). I'm happy to type azule_url as string | null if you think it is appropriate?
There was a problem hiding this comment.
Interesting; there must be downstream handling (i.e. type narrowing) on the possible null values? Let's keep it as it was (string | null) but remove the TODO comments.
| file_name: string; | ||
| file_size: number; | ||
| file_type: string; | ||
| url: string; |
There was a problem hiding this comment.
Removed url to ensure it isn't used accidentally (generates TS error if it is).
MillenniumFalconMechanic
left a comment
MillenniumFalconMechanic
left a comment
There was a problem hiding this comment.
LGTM, thanks F! Two follow-ups before we merge:
- See my comment regarding
string | null. - I've eyeballed the code but can you confirm LungMAP inherits these updates from HCA?
Closes #4575.
This pull request updates how file download URLs are handled in both the Anvil-CMG and HCA-DCP entities and view model builders. The main change is replacing the previous
urlproperty with a newazul_urlproperty to standardize access to file download links.Entity interface updates:
urlproperty withazul_urlin theFileEntityinterface inapp/apis/azul/anvil-cmg/common/entities.tsto reflect the new naming convention. [1] [2]urlproperty withazul_urlin theFileResponseinterface inapp/apis/azul/hca-dcp/common/entities.tsfor consistency across APIs. [1] [2]View model builder updates:
buildFileDownloadinapp/viewModelBuilders/azul/anvil-cmg/common/viewModelBuilders.tsto use the newazul_urlproperty instead ofurlwhen constructing file download objects.buildFileDownloadinapp/viewModelBuilders/azul/hca-dcp/common/viewModelBuilders.tsto useazul_urlinstead ofurl, ensuring the view models reference the correct property.