← Back to team overview

widelands-dev team mailing list archive

Re: [Merge] lp:~widelands-dev/widelands-website/move_minimaps into lp:widelands-website

 

I think it would be better if all related code in forms.py depends on 'upload_to', as well for the minimap as also for the mapfile. There are several places where "/wlmaps/maps" or "/wlmaps/minimaps" are hardcoded as strings: 

1. Storing the mapfile for wl_map_info
2. Moving the minimap
3. The string for minimap which is stored in the db

If all these places uses the strings of the appropriate 'upload_to' attribute in models.py, the code would be less error-prone, imho. But using the string from 'upload_to' couldn't be implemented in this branch because os.path.join wouldn't work on some places right now:

minimap_path = Map._meta.get_field('minimap').upload_to  #="/wlmaps/minimaps"
os.path.join(MEDIA_ROOT, minimaps_path, file.name + '.png' ) 

In this case MEDIA_ROOT gets omitted because minimap_path has a leading slash -> fail

But my former plan will fix this. In short:
1. apply this branch (moving minimaps from wlmaps/maps/ to wlmaps/minimaps on future uploads)
2. Next branch (correct_old_upload_paths): Contains two admin actions
2.1 Move old minimaps stored in wlmaps/maps to wlmaps/minimaps/ including fixes in the database
2.2.Correct old paths in the database for the maps (f.e. from
    /var/www/django_projects/wlwebsite/code/widelands/media//wlmaps/maps/Gestrandet.wmf
    to
    wlmaps/maps/Gestrandet.wmf
This branch will also change the leading slash for minimaps and maps in the database. As soon these changes are applied the maps page on the website doesn't show the images for the minimaps anymore. But this will be fixed in the next branch.
3. Revert the 2. branch (after the admin actions are executed they are useless)
4. Apply branch "fix_wlimages" (has to be created): Makes the images for maps visible again and change the 'upload_to' to have not the leading slash anymore. If this is done we could also change forms.py to use 'upload_to' in all places where it is needed (see first section in this post)

Yes, i know it looks very complicated, but i haven't found a better solution yet. Of course we could also leave all as it is right now, just because it works... but doing this cleanup may prevent future issues. If one thinks this is useless work, say it now :-)

In case of making it so, we may have to disable map uploading for at least 30 minutes i think.
-- 
https://code.launchpad.net/~widelands-dev/widelands-website/move_minimaps/+merge/303493
Your team Widelands Developers is subscribed to branch lp:widelands-website.


References