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.

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

oaisys: change etdms date source to graduation date as per LAC spec #2298 #2510

Merged
merged 4 commits into from
Sep 29, 2021

Conversation

jefferya
Copy link
Contributor

@jefferya jefferya commented Sep 10, 2021

Context

Update decorator for theses that provides the specified LAC etdms date and format replacing the the current date_accepted and created_at dates that don't meet the LAC requirements

Related to #2298

What's New

@github-actions
Copy link

github-actions bot commented Sep 10, 2021

1 Warning
⚠️ This CHANGELOG entry seems quite short. Reviewers, please check that it contains enough information, and request expansion if it seems unreasonably brief.

Generated by 🚫 Danger

Copy link
Member

@pgwillia pgwillia left a comment

Choose a reason for hiding this comment

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

I think you're on the right track with this.

Comment on lines 554 to 555
spring_num: '06'
fall_num: '11'
Copy link
Member

Choose a reason for hiding this comment

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

Seems like it might be redundant with

# Metadata team prefers we store and use a number (e.g. '06' or '11')
# to represent the graduation term (e.g. Spring or Fall)
# This TERMS constant is used by the graduation term dropdown on the deposit form,
# mapping the string label to the number value that we wish to use.
TERMS = [
[I18n.t('admin.theses.graduation_terms.spring'), '06'],
[I18n.t('admin.theses.graduation_terms.fall'), '11']
].freeze

Can we consolidate to use one or the other?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My struggle here is developing a mental model how Thesis and DraftThesis objects relate. Is there a document that describes the two at a high level contrasting the differences? My naive first assumption was that the Thesis and DraftThesis would be very similar models with only minor differences owning to the draftiness of a DraftThesis. Looking through the codebase, I see multiple areas that transition dates (e.g., graduation date) from a text representation to a number (string or numerical datatype) version, for example, the conversions ( 6 => Spring, '06 (Spring)' => '06', 'Spring' => '06', '06' => 'Spring'). The above code in the context of DraftThesis is using the text 06 (Spring) whereas the data in the Thesis context is Spring (no round brackets etc.) so I unsure if a consolidation can happen due to differences in the string. I like the idea of consolidation but I not confident I have an understanding of the nuances in the aforementioned and following locations spread across different models.

SEASON = {
6 => I18n.t('items.thesis.graduation_terms.spring'),
11 => I18n.t('items.thesis.graduation_terms.fall')
}.freeze

graduation_term = '11' if graduation_term_string == 'Fall'
graduation_term = '06' if graduation_term_string == 'Spring'

spring: '06 (Spring)'
fall: '11 (Fall)'

Copy link
Member

@pgwillia pgwillia Sep 28, 2021

Choose a reason for hiding this comment

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

Effort was made so that the underlying data was consistent: ualbertalib/metadata#383 and I believe this is still the case. I reviewed the graduation_date on staging and can confirm that all are YYYY or YYYY-MM. So now I'm curious where the Fall YYYY or Spring YYYY came from. It might be possible to use the graduation date directly.

The public presentation of these dates (Fall 2017) was discussed #1003 where OAI was out of scope.
The admin deposit interface should show the number (06 for spring and 11 for fall) #804 #709

Copy link
Member

Choose a reason for hiding this comment

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

I can see that our seeds data is spitting out Fall. That looks like a bad rebase b0f53cb. It was fixed before that commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thx for checking the underlying DB. I can confirm, the graduation_date appears to work as the DB contents conforms to the LAC format. The Fall/Spring YYYY was from me looking at the DB created from the seeds.rb file and a conversation with Mariana on Theses inputs which I'd falsely assumed aligned with the DB storage format. I'm updating the seeds.rb

@jefferya jefferya changed the title [WIP] add decorator for theses to provide LAC etdms dates #2298 oaisys: change etdms date source to graduation date as per LAC spec #2298 Sep 29, 2021
Copy link
Member

@pgwillia pgwillia left a comment

Choose a reason for hiding this comment

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

👍

@@ -19,8 +19,7 @@ def type
end

def date
# TODO: need to talk to metadata about fallback if date accepted is not present, which it isn't for legacy theses
object.date_accepted || object.created_at
object.graduation_date
Copy link
Member

Choose a reason for hiding this comment

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

This should be present so feel pretty good about using graduation_date

validates :graduation_date, presence: true

@jefferya jefferya merged commit 69afc32 into master Sep 29, 2021
@jefferya jefferya deleted the jefferya/2298_oai_etdms_date branch September 29, 2021 22:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants