Mission run refactor #864
Mission run refactor #864
Conversation
|
🔔 Changes in database folder detected 🔔 |
|
In this PR the Area attribute is currently nullable whilst the AssetCode value is used instead. In the future AssetCode should be removed in favour of Area, which contains the AssetCode, but populating the Area values is left for another PR. |
andchiind
force-pushed
the
mission-run-refactor
branch
from
4989468 to
eb445d8
Compare
| { | ||
| [Key] | ||
| [DatabaseGenerated(DatabaseGeneratedOption.Identity)] | ||
| public string Id { get; set; } | ||
|
|
||
| [MaxLength(200)] | ||
| public int? EchoMissionId { get; set; } | ||
| public string? MissionId { get; set; } |
There was a problem hiding this comment.
| public string? MissionId { get; set; } | |
| public MissionDefinition MissionDefinition { get; set; } |
I don't think this should be optional, please correct me if I'm wrong. Also if we are pointing to the ID it makes more sense to point straight to the model.
There was a problem hiding this comment.
In addition, I think this field will be breaking for all old missions in the database.
There was a problem hiding this comment.
You could set the MissionDefinition as optional for now such that this does not break compatability with old missions and then create an issue which is to add a MissionDefinition for all old missions such that this one may be made required.
There was a problem hiding this comment.
I also think it would be nice to include MissionDefinition here. See this comment for why I have not yet implemented it: #864 (comment)
andchiind
force-pushed
the
mission-run-refactor
branch
4 times, most recently
from
9348cbe to
54782ac
Compare
andchiind
force-pushed
the
mission-run-refactor
branch
from
54782ac to
5a7eb15
Compare
andchiind
dismissed
aeshub’s stale review
Requests have been addressed while reviewer is on holiday
|
/UpdateDatabase |
|
⛔ Cannot update database until the Pull Request is approved! ⛔ |
|
/UpdateDatabase |
|
⛔ Cannot update database until the Pull Request is approved! ⛔ |
|
/UpdateDatabase |
|
⛔ Cannot update database until the Pull Request is approved! ⛔ |
|
Merged branch in another PR |
No description provided.