Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adds image and image_alternative_text fields to Alerts #670

Merged
merged 3 commits into from
Sep 14, 2023
Merged

Adds image and image_alternative_text fields to Alerts #670

merged 3 commits into from
Sep 14, 2023

Conversation

ErinLMoore
Copy link
Contributor

Summary of changes

Asana Ticket: 🍎 Add image fields to /alerts API responses

Adds experimental Image and Image Alternative Text fields to Alerts. Image Alternative Text is a translated string type, which we already handle. Image is a localized image type, which works very similarly to translated strings. Both fields are nullable.

Reference for localized image type: https://github.com/google/transit/blob/master/gtfs-realtime/spec/en/reference.md#message-translatedimage

@ErinLMoore ErinLMoore self-assigned this Sep 6, 2023
@github-actions
Copy link

github-actions bot commented Sep 6, 2023

Coverage of commit f38d2a8

Summary coverage rate:
  lines......: 89.6% (4044 of 4511 lines)
  functions..: 70.8% (2217 of 3133 functions)
  branches...: no data found

Files changed coverage rate:
                                                                        |Lines       |Functions  |Branches    
  Filename                                                              |Rate     Num|Rate    Num|Rate     Num
  ============================================================================================================
  apps/api_web/lib/api_web/controllers/alert_controller.ex              |97.9%     97| 100%    20|    -      0
  apps/api_web/lib/api_web/views/alert_view.ex                          | 100%     23|80.0%    60|    -      0
  apps/model/lib/model/alert.ex                                         | 0.0%      3| 0.0%    11|    -      0
  apps/parse/lib/parse/alerts.ex                                        |93.6%     47| 100%    19|    -      0

Download coverage report

@ErinLMoore ErinLMoore marked this pull request as ready for review September 7, 2023 13:28
@ErinLMoore ErinLMoore requested review from arkadyan, a team and bfauble September 7, 2023 13:28
paulswartz
paulswartz reviewed Sep 14, 2023
View reviewed changes
apps/parse/lib/parse/alerts.ex Show resolved Hide resolved
apps/parse/test/parse/alerts_test.exs Show resolved Hide resolved
apps/parse/test/parse/alerts_test.exs
"timeframe_text" => []
}
]
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: should we have a test that we prefer an en image over a generic image?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

followup: should we have a test that we prefer an image with no language over a non-en language?

@github-actions
Copy link

Coverage of commit e79c9ef

Summary coverage rate:
  lines......: 89.6% (4044 of 4513 lines)
  functions..: 70.8% (2217 of 3133 functions)
  branches...: no data found

Files changed coverage rate:
                                                                        |Lines       |Functions  |Branches    
  Filename                                                              |Rate     Num|Rate    Num|Rate     Num
  ============================================================================================================
  apps/api_web/lib/api_web/controllers/alert_controller.ex              |97.9%     97| 100%    20|    -      0
  apps/api_web/lib/api_web/views/alert_view.ex                          | 100%     23|80.0%    60|    -      0
  apps/model/lib/model/alert.ex                                         | 0.0%      3| 0.0%    11|    -      0
  apps/parse/lib/parse/alerts.ex                                        |91.8%     49| 100%    19|    -      0

Download coverage report

@github-actions
Copy link

Coverage of commit 3e9797a

Summary coverage rate:
  lines......: 89.6% (4047 of 4515 lines)
  functions..: 70.8% (2217 of 3133 functions)
  branches...: no data found

Files changed coverage rate:
                                                                        |Lines       |Functions  |Branches    
  Filename                                                              |Rate     Num|Rate    Num|Rate     Num
  ============================================================================================================
  apps/api_web/lib/api_web/controllers/alert_controller.ex              |97.9%     97| 100%    20|    -      0
  apps/api_web/lib/api_web/views/alert_view.ex                          | 100%     23|80.0%    60|    -      0
  apps/model/lib/model/alert.ex                                         | 0.0%      3| 0.0%    11|    -      0
  apps/parse/lib/parse/alerts.ex                                        |92.2%     51| 100%    19|    -      0

Download coverage report

@github-actions
Copy link

Coverage of commit 35760c6

Summary coverage rate:
  lines......: 89.6% (4046 of 4515 lines)
  functions..: 70.8% (2217 of 3133 functions)
  branches...: no data found

Files changed coverage rate:
                                                                        |Lines       |Functions  |Branches    
  Filename                                                              |Rate     Num|Rate    Num|Rate     Num
  ============================================================================================================
  apps/api_web/lib/api_web/controllers/alert_controller.ex              |97.9%     97| 100%    20|    -      0
  apps/api_web/lib/api_web/views/alert_view.ex                          | 100%     23|80.0%    60|    -      0
  apps/model/lib/model/alert.ex                                         | 0.0%      3| 0.0%    11|    -      0
  apps/parse/lib/parse/alerts.ex                                        |92.2%     51| 100%    19|    -      0

Download coverage report

@ErinLMoore ErinLMoore merged commit d63f9e3 into master Sep 14, 2023
7 checks passed
@ErinLMoore ErinLMoore deleted the elm-add-image-fields branch September 14, 2023 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@paulswartz paulswartz paulswartz approved these changes

@arkadyan arkadyan Awaiting requested review from arkadyan

@bfauble bfauble Awaiting requested review from bfauble bfauble was automatically assigned from mbta/transit-data

Assignees

@ErinLMoore ErinLMoore

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ErinLMoore @paulswartz