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

Configuration Management #755

Merged
merged 36 commits into from
Jan 6, 2021
Merged

Conversation

bastihav
Copy link
Collaborator

@bastihav bastihav commented Dec 3, 2020

Description

  • Implements configuration management.
  • course types, countries, languages, validation, current semester are now fetched from the backend
  • implements frontend validation

WIP:

  • we should probably add e2e tests for frontendValidation

Dependency update

  • List all new dependencies (delete if no additions)

Type of change (remove all that don't apply)

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Manually tested all use cases this could alter

Test Configuration:

  • OS: Windows Mac Linux

  • Browser: Firefox Chrome Safari Chromium

  • Frontend: (remove all that don't apply)

    • Development build
    • Production build
    • Dockerized production build
  • Backend: (remove all that don't apply)

    • deployed development build

Checklist: (remove all that don't apply)

  • I added corresponding E2E tests (especially for bugfixes)
  • I added corresponding unit tests (if API code)
  • I have performed a self-review of my own code
  • My changes generate no new linting warnings or console warnings
  • I have updated the CHANGELOG.md to include any changes made in this PR (add WIP to the top, if there is none already)

@bastihav bastihav added the vue label Dec 3, 2020
@bastihav bastihav marked this pull request as ready for review December 4, 2020 14:55
@bastihav bastihav changed the title Feature/configuration management Configuration Management Dec 4, 2020
@bastihav
Copy link
Collaborator Author

bastihav commented Dec 4, 2020

image

the merge introduced some ugly styling

@tobias-wiese
Copy link
Contributor

image

the merge introduced some ugly styling

this should be fixed

Copy link
Contributor

@tobias-wiese tobias-wiese left a comment

Choose a reason for hiding this comment

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

Overall that is great work.
For the E2E we should propagate the id from the parent component into the base-input component for properly getting them via Cypress. Otherwise, it looks like the following:
grafik

I would suggest something like
In the parent:
<base-input identifier="WhatEverIdYouWant" ..../>
and stting the input field's id via the prop in the BaseInput.vue
<input :id="identifier" .../>

Additionally we must not forget to rework the error message assertation in the E2E tests

@tobias-wiese
Copy link
Contributor

Consider closing #354 on merge (or connecting it)

@tobias-wiese
Copy link
Contributor

Overall that is great work.
For the E2E we should propagate the id from the parent component into the base-input component for properly getting them via Cypress. Otherwise, it looks like the following:
grafik

I would suggest something like
In the parent:
<base-input identifier="WhatEverIdYouWant" ..../>
and stting the input field's id via the prop in the BaseInput.vue
<input :id="identifier" .../>

Additionally we must not forget to rework the error message assertation in the E2E tests

should be fixed now

@bastihav bastihav merged commit 1d58d88 into develop Jan 6, 2021
@bastihav bastihav deleted the feature/configuration_management branch January 6, 2021 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants