Skip to content

Remove some unused methods and add specs for LogDocumentError class#6053

Merged
mitchellhenke merged 2 commits intomainfrom
mitchellhenke/improve-test-coverage
Mar 14, 2022
Merged

Remove some unused methods and add specs for LogDocumentError class#6053
mitchellhenke merged 2 commits intomainfrom
mitchellhenke/improve-test-coverage

Conversation

@mitchellhenke
Copy link
Contributor

No description provided.

Mitchell Henke added 2 commits March 11, 2022 15:26
changelog: Internal, Testing, Improve test coverage
@mitchellhenke mitchellhenke force-pushed the mitchellhenke/improve-test-coverage branch from 49e80f6 to b9e9858 Compare March 11, 2022 21:33
Comment on lines -202 to -204
def aal
Saml::Idp::Constants::AUTHN_CONTEXT_CLASSREF_TO_AAL[aal_values.sort.max]
end
Copy link
Contributor

Choose a reason for hiding this comment

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

this is very surprising to me that it's not used...? are we getting aal elsewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I18n.t('errors.messages.confirmation_period_expired', period: confirmation_period)
end

def confirmation_period
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was used, but only in tests, the live usage is actually in ConfirmationEmailPresenter

@mitchellhenke mitchellhenke marked this pull request as ready for review March 11, 2022 21:57
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

@mitchellhenke mitchellhenke merged commit ede0486 into main Mar 14, 2022
@mitchellhenke mitchellhenke deleted the mitchellhenke/improve-test-coverage branch March 14, 2022 13:56
@aduth aduth mentioned this pull request Mar 15, 2022
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