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

Extension does not consider first two semesters #99

Open
1 of 2 tasks
simonbaumeler opened this issue Oct 6, 2022 · 1 comment
Open
1 of 2 tasks

Extension does not consider first two semesters #99

simonbaumeler opened this issue Oct 6, 2022 · 1 comment
Labels
bug Something isn't working

Comments

@simonbaumeler
Copy link

Describe the bug
The diagram starts in the third semester instead of the first.

Expected behavior
My earned and planed credits are shown correctly on the diagram.

Actual behavior
It starts displaying my credits from the third semester instead of from the first semester.

Steps to Reproduce
[ Tell us how to reproduce the bug. ]
For me its just opening the "Meine Anmeldungen" page. I started in the spring semester so i dont know if that could be a factor but i know that my colleagues which also started in the same semester do not have this issue.

Screenshots
image
Contributing (please choose one)

  • I'd like to help implementing this feature!
  • I don't have the time to work on this.
@simonbaumeler simonbaumeler added the bug Something isn't working label Oct 6, 2022
@rogerKaelin
Copy link

The issue is probably caused by improper first semester detection. Spring semesters are not recognized as possible first semesters and Info modules like the INFO_ABEND are not ignored as they should be.

Old bad code:

firstModule = anlasslistApiResponse.items
            .slice()
            .reverse()
            .find(modul => ModuleParser.isAutumnSemester(modul.anlassnumber) != undefined);

With following method further up:

    /**
     * Check if a module was done in Autumn.
     * Modules are marked with 'H' for 'Herbstsemester' (autumn)
     * or 'F' for 'Frühlingssemester' (spring).
     */
    isAutumnSemester: (hsluModuleName) => {
        const includesH = hsluModuleName.split('.')[2].includes('H');
        const includesF = hsluModuleName.split('.')[2].includes('F');
        return includesH || includesF ? includesH : undefined;
    },

New and improved code:

let firstSpringModule = anlasslistApiResponse.items
            .slice()
            .reverse()
            .find(modul => ModuleParser.isSpringSemester(modul.anlassnumber));

let firstAutumnModule = anlasslistApiResponse.items
            .slice()
            .reverse()
            .find(modul => ModuleParser.isAutumnSemester(modul.anlassnumber));

if (new Date(firstAutumnModule.from).getFullYear() < (new Date(firstSpringModule.from).getFullYear())) {
            firstModule = firstAutumnModule
} else {
            firstModule = firstSpringModule
}

And further up I needed to change or add following methods:

    /**
     * Check if a module is an info module
     * Info modules contain INFO in their name
     */
    isNotInfoSemester: (hsluModuleName) => {
        return !hsluModuleName.includes('INFO')
    },

    /**
     * Check if a module was done in Autumn.
     * Modules are marked with 'H' for 'Herbstsemester' (autumn)
     */
    isAutumnSemester: (hsluModuleName) => {
        return hsluModuleName.split('.')[2].includes('H') && ModuleParser.isNotInfoSemester(hsluModuleName);
    },

    /**
     * Check if a module was done in Spring.
     * Modules are marked with 'F' for 'Frühlingssemester' (spring).
     */
    isSpringSemester: (hsluModuleName) => {
        return hsluModuleName.split('.')[2].includes('F') && ModuleParser.isNotInfoSemester(hsluModuleName);
    },

-> I've fixed it locally but couldn't yet be bothered to push it.

rogerKaelin added a commit to rogerKaelin/hslu-simple-mep-results that referenced this issue Oct 18, 2022
…nt-1283029378

-Fixed the first semester calculation -> Check comment for detailed description of fix
-Also general cleanup as my ide warned me about simple things
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants