fix: adding demoted, migration, API change for upcoming event

[maze-runnar]

Fixes fossasia/open-event-frontend#4963
continuing #7588

Checklist

• I have read the Contribution & Best practices Guide and my PR follows them.
• My branch is up-to-date with the Upstream development branch.
• The unit tests pass locally with my changes
• I have added tests that prove my fix is effective or that my feature works
• I have added necessary documentation (if appropriate)

• All the functions created/modified in this PR contain relevant docstrings.

[iamareebjamal]

Where is server_default?

This migration will fail. Please test the changes carefully

[maze-runnar]

This migration will fail. Please test the changes carefully

But I have tested it with frontend, it's working. did server_default is not added itself in autogenerated migration?

[iamareebjamal]

No

[iamareebjamal]

This won't work, the first command itself will fail

[iamareebjamal]

If you drop the column is_demoted, how can you set it to True? And even if you could, would you make all events demoted? Now, no event is being shown on frontend, WOW

Please be careful and think about these things :/

[maze-runnar]

I thought about it. But you have said before once that downgrade should be opposite of upgrade 😕

[codecov]

Codecov Report

Merging #7589 (02bf1c1) into development (3aed6f5) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##           development    #7589   +/-   ##
============================================
  Coverage        67.20%   67.20%           
============================================
  Files              271      271           
  Lines            13732    13734    +2     
============================================
+ Hits              9228     9230    +2     
  Misses            4504     4504           

Impacted Files	Coverage Δ
app/api/events.py	59.27% <ø> (ø)
app/api/schema/events.py	96.99% <100.00%> (+0.02%)	⬆️
app/models/event.py	82.60% <100.00%> (+0.06%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3aed6f5...3e64875. Read the comment docs.

[iamareebjamal]

Suggested change

	op.add_column('events', sa.Column('is_demoted', sa.Boolean(), nullable=False, server_default=False))
	op.add_column('events', sa.Column('is_demoted', sa.Boolean(), nullable=False, server_default='False'))

server_default needs to be quoted

[iamareebjamal]

Suggested change

op.execute("UPDATE events SET is_demoted=False")

Handled by server default

[iamareebjamal]

But you have said before once that downgrade should be opposite of upgrade

Then sorry, correct definition is downgrade should restore the DB structure and state to where it was before migration.

So, you drop the column, it is already restored
Once you dropped the column, it is not there so how can you set demoted to true?

Secondly, let's say you set default demoted to false, it makes sense, would you restore demoted to true on downgrade? It doesn't make sense, it'll make all events demoted. All events were never demoted, so demoted was never = true, so it is not restoring to correct state

In case of making custom forms public

Original State:
All forms are private by default, maybe some custom fields are made public by users

Migration:
Make all forms public - WRONG

Loss of state

Correct Migration:
Make speaker fields x, y, z public
Make session fields x, y, z public

Downgrade:
Make all fields private - WRONG, orignal state may have some public fields which users made

Data Loss

Thus, only make those fields private which you made public to restore to the original state.

Make speaker fields x, y, z private
Make session fields x, y, z private

Development is not just taking an idea that you heard previously and applying it without thought.
The reason of the idea is important. Goal is to revert to the original state of database, not just write opposite commands of the migration