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

Create hasTokenCounter attribute to assessment_config #1062

Merged
merged 6 commits into from
Feb 13, 2024
Merged

Create hasTokenCounter attribute to assessment_config #1062

merged 6 commits into from
Feb 13, 2024

Conversation

jayjay19630
Copy link
Contributor

@jayjay19630 jayjay19630 commented Feb 12, 2024

Create hasTokenCounter attribute to assessment_config

  • Create hasTokenCounter attribute to assessment_config
  • Render hasTokenCounter upon GET request

@coveralls
Copy link

coveralls commented Feb 12, 2024

Coverage Status

coverage: 95.295%. remained the same
when pulling 8e6c049 on jayjay19630:master
into dba4f95 on source-academy:master.

Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

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

I understand that the other columns are also defined the same way (non-null, but optional) – this may or may not have been an error, let me get back to you after I check with the original implementers.

Please remind me if I don't get back to you after a couple days.

Thanks for working on this!

@RichDom2185
Copy link
Member

Oh, I realised some tests are failing, could you fix them as well? We try to keep the coverage ≥ 95%. Thanks!

@RichDom2185
Copy link
Member

Corresponding frontend PR: source-academy/frontend#2775

Copy link
Contributor

@sayomaki sayomaki left a comment

Choose a reason for hiding this comment

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

Looks good to me, keep up the good effort and contributions!

@jayjay19630 jayjay19630 merged commit 18a71d3 into source-academy:master Feb 13, 2024
2 checks passed
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.

5 participants