Support paging of figgy born ephemera - εφήμερα objects#3239
Conversation
christinach
force-pushed
the
3230-ephemera-index
branch
from
88b2971 to
b92a011
Compare
christinach
force-pushed
the
3230-ephemera-index
branch
from
b92a011 to
10028bc
Compare
|
Indexed ephemera on catalog-staging figgy ephemera count: 390 |
sandbergja
left a comment
sandbergja
left a comment
There was a problem hiding this comment.
Nice, thanks @christinach ! Two questions/suggestions -- neither one is a blocker
|
|
||
| #[derive(Deserialize, Debug)] | ||
| pub struct Pages { | ||
| // pub current_page: Option<u32>, |
There was a problem hiding this comment.
Do you think we could remove these commented-out fields?
| .map(|m| m.pages.total_pages) | ||
| .unwrap_or(1); | ||
|
|
||
| all_ids.extend(response.data.into_iter().map(|item| item.id)); |
There was a problem hiding this comment.
Since response.data.into_iter().map(|item| item.id)) is repeated multiple times, do you think it would be worth it to pull it out as a method in an impl block for the FoldersResponse struct?
| page_number += 1; | ||
|
|
||
| let response = client.get_folder_data(page_number).await?; | ||
| all_ids.extend(response.data.into_iter().map(|item| item.id)); |
|
@sandbergja thanks for reviewing can you please check again? |
sandbergja
left a comment
sandbergja
left a comment
There was a problem hiding this comment.
Thanks, @christinach ! Looks great! 🐬
I had some (non-blocking) suggestions about the signature of the new method, otherwise looks great!
| pub fn response_ids( | ||
| response: FoldersResponse, | ||
| ) -> std::iter::Map< | ||
| std::vec::IntoIter<BornDigitalCollection>, | ||
| impl FnMut(BornDigitalCollection) -> String, | ||
| > { | ||
| response.data.into_iter().map(|item| item.id) | ||
| } |
There was a problem hiding this comment.
Two ideas to simplify the signature a bit:
- If we use
&self, we can replaceFoldersResponse::response_ids(response)withresponse.response_idsbelow - We don't necessarily have to be as specific about the return type
| pub fn response_ids( | |
| response: FoldersResponse, | |
| ) -> std::iter::Map< | |
| std::vec::IntoIter<BornDigitalCollection>, | |
| impl FnMut(BornDigitalCollection) -> String, | |
| > { | |
| response.data.into_iter().map(|item| item.id) | |
| } | |
| pub fn response_ids( | |
| &self | |
| ) -> impl Iterator<Item=String> | |
| { | |
| self.data.into_iter().map(|item| item.id.clone()) | |
| } |
There was a problem hiding this comment.
@sandbergja thanks I will open a new Pr with your suggestion
3 participants
related to [#3230]