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 global initial version constant and use it for initializing all the versioned entities #10274

Open
DubeySandeep opened this issue Aug 11, 2020 · 4 comments
Labels
architectural refactor Architectural refactors that clean up the codebase enhancement Label to indicate an issue is a feature/improvement Impact: Low -- DO NOT WORK ON THIS YET Postponing for now, since it doesn't affect users much. Work: Low Solution is known and broken into good-first-issue-sized chunks.

Comments

@DubeySandeep
Copy link
Member

Currently, the initial value for the version number is hardcoded in the domain layer for all the entities which make it open for any entity to add incorrect initial version number. This issue focus on defining a global constant initial entity version (Which will be zero) and use this constant in all the files.

Few places in the codebase where we have hardcoded the initial version:

Pointers (to start with):

  • Check all the models which are made out of VersionedModel.
  • Check how the new models are created for these versionsedModel
  • Add constant for the version number.
@vojtechjelinek vojtechjelinek added this to To do in Data handling Sep 23, 2020
@vojtechjelinek vojtechjelinek moved this from TODO (critical) to TODO (improvement/feature) in Data handling Oct 13, 2020
@vojtechjelinek vojtechjelinek moved this from TODO (improvement/feature) to TODO (critical) in Data handling Oct 22, 2020
@vojtechjelinek vojtechjelinek moved this from TODO (critical) to In progress in Data handling Oct 27, 2020
@vojtechjelinek vojtechjelinek moved this from In progress to TODO (bug) in Data handling Feb 9, 2021
@seanlip
Copy link
Member

seanlip commented Sep 19, 2022

This is suitable for a good first issue (since it can be done with limited context), but is on the slightly harder side so is probably better taken up by contributors who have previous experience with refactoring a codebase. The plan should be:

  • Define an INITIAL_ENTITY_VERSION constant in feconf.py.
  • Find all places where that version is set manually when creating domain objects, and change that to use the default initial entity version instead. (There may be other places besides the ones @DubeySandeep listed.)

Note: If you see any conflicts (e.g. one place uses 0 and another uses 1 for the initial entity version), please comment on this issue and add links to those places so that we can advise on what to do here. Thanks!

Additional note: See #20301 for a prior PR that wasn't completed (but that might be a good starting point).

@seanlip seanlip removed this from TODO (bug) in Data handling Oct 11, 2022
@seanlip seanlip added enhancement Label to indicate an issue is a feature/improvement Impact: Low -- DO NOT WORK ON THIS YET Postponing for now, since it doesn't affect users much. Work: Low Solution is known and broken into good-first-issue-sized chunks. labels Jan 5, 2023
@Chiranjeev-Kartik
Copy link

I would be happy to work on this issue. Please assign it to me.

@seanlip
Copy link
Member

seanlip commented Feb 3, 2023

@Chiranjeev-Kartik Per the guidance at https://github.com/oppia/oppia/wiki/Contributing-code-to-Oppia#choosing-a-good-first-issue, please provide an explanation of what your PR will do (with names of files you're changing, what you plan to change in each file, etc.). If it looks good, we can assign you to this issue.

For this issue I would suggest making a list of the specific places in the code you would change per my previous comment, explain how you found them, and how you know that that's the complete list.

Please also follow the other instructions on that wiki page if you have not yet done so. Thanks!

@kumarankit999
Copy link

Should i work on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
architectural refactor Architectural refactors that clean up the codebase enhancement Label to indicate an issue is a feature/improvement Impact: Low -- DO NOT WORK ON THIS YET Postponing for now, since it doesn't affect users much. Work: Low Solution is known and broken into good-first-issue-sized chunks.
Projects
Status: Todo
Development

No branches or pull requests

6 participants