Commit 114ea1cf authored by Florent Chehab's avatar Florent Chehab

Merge branch 'clean_mydotdotdot_in_code' into 'master'

Cleaned my.... and custom in code; use of super

See merge request !55
parents 9a61b32d 5c580b14
Pipeline #35535 passed with stages
in 5 minutes and 14 seconds
......@@ -38,14 +38,17 @@ class BasicModuleSerializer(MyModelVersionnedSerializer):
Custom serializer that performs checks on the Basic module filed
"""
def my_validate(self, attrs):
def validate(self, attrs):
"""
Checks that the useful_links have been filled properly
"""
attrs = super().validate(attrs)
content = {"useful_links": attrs["useful_links"]}
config = {"useful_links": USEFULL_LINKS_CONFIG}
validate_content_against_config(config, content)
return attrs
class Meta:
......
......@@ -6,6 +6,7 @@ from backend_app.permissions import is_moderation_required
from backend_app.utils import get_user_level
from rest_framework import serializers
from rest_framework.validators import ValidationError
from shared.obj_moderation_permission import DEFAULT_OBJ_MODERATION_LV
from .myModel import MyModel
from .pendingModeration import PendingModeration, PendingModerationSerializer
......@@ -88,11 +89,11 @@ class MyModelSerializer(MySerializerWithJSON):
class Meta:
model = MyModel
def my_validate(self, attrs):
def get_user_from_request(self):
"""
Function to be redefined in the subclasses to validate class fields.
Function to retrieve the user from the request
"""
return attrs
return self.context["request"].user
def validate(self, attrs):
"""
......@@ -100,30 +101,35 @@ class MyModelSerializer(MySerializerWithJSON):
TODO unit test this
"""
# Enforce fields values based on request
self.user = self.context["request"].user
self.user_level = get_user_level(self.user)
if "obj_moderation_level" in attrs:
requested_obj_moder_lv = attrs["obj_moderation_level"]
if requested_obj_moder_lv > self.user_level:
try:
user_level = get_user_level(self.get_user_from_request())
except KeyError:
# if for some reason we don't have the user in the request
# we set the level to the default one
# this can occur during testing.
user_level = DEFAULT_OBJ_MODERATION_LV
if requested_obj_moder_lv > user_level:
raise ValidationError(
"You can't request moderation for a higher rank than you."
)
return self.my_validate(attrs)
return attrs
def set_model_attr_no_moder(self, moderated_and_updated):
def set_model_attr_no_moder(self, user, moderated_and_updated):
"""
TODO
TODO: rename ?
"""
now = timezone.now()
self.override_validated_data({"moderated_by": self.user, "moderated_on": now})
self.override_validated_data({"moderated_by": user, "moderated_on": now})
if moderated_and_updated:
self.override_validated_data({"updated_by": self.user, "updated_on": now})
self.override_validated_data({"updated_by": user, "updated_on": now})
def clean_validated_data(self):
"""
......@@ -138,34 +144,32 @@ class MyModelSerializer(MySerializerWithJSON):
for key in new_data:
self.validated_data[key] = new_data[key]
def my_pre_save(self):
def do_before_save(self):
"""
TODO, Analyse if usefull
Action to perform before saving a model
"""
pass
def save(self, *args, **kwargs):
"""
TODO analyse, usefull ?
"""
return self.my_save(*args, **kwargs)
def my_save(self, *args, **kwargs):
"""
Function that handles all the moderation in a smart way.
Nothing has to be done to tell that we won't the data to be moderated, it is detected automatically.
Nothing has to be done to tell that we won't the data to be moderated,
it is detected automatically.
"""
user = self.context["request"].user
user_level = get_user_level(user)
self.clean_validated_data()
self.my_pre_save()
self.do_before_save()
ct = ContentType.objects.get_for_model(self.Meta.model)
if is_moderation_required(
self.get_model_config(), self.instance, self.user, self.user_level
self.get_model_config(), self.instance, user, user_level
):
if self.instance is None: # we need to create the main model
# Store the user for squashing data in versions models
self.validated_data.updated_by = self.user
self.validated_data.updated_by = user
self.instance = super().save(*args, **kwargs)
data_to_save = dict()
......@@ -185,7 +189,7 @@ class MyModelSerializer(MySerializerWithJSON):
object_id=self.instance.pk,
defaults={
"updated_on": timezone.now(),
"updated_by": self.user,
"updated_by": user,
"new_object": data_to_save,
},
)
......@@ -195,7 +199,7 @@ class MyModelSerializer(MySerializerWithJSON):
moderated_and_updated = True
if self.instance is None:
self.set_model_attr_no_moder(moderated_and_updated)
self.set_model_attr_no_moder(user, moderated_and_updated)
return super().save(*args, **kwargs)
else:
try:
......@@ -221,5 +225,5 @@ class MyModelSerializer(MySerializerWithJSON):
except PendingModeration.DoesNotExist:
pass
self.set_model_attr_no_moder(moderated_and_updated)
self.set_model_attr_no_moder(user, moderated_and_updated)
return super().save(*args, **kwargs)
......@@ -65,7 +65,7 @@ class MyModelVersionnedSerializer(MyModelSerializer):
"""
with reversion.create_revision():
res = self.my_save(*args, **kwargs)
res = super().save(*args, **kwargs)
reversion.set_user(res.updated_by)
new_revision_saved.send(sender=self.__class__, obj=self.instance)
return res
......
......@@ -40,13 +40,6 @@ class MyModelViewSet(viewsets.ModelViewSet):
Extended default rest framework behavior
to prefetch some table and enhance performances
"""
self.my_model_queryset = self.queryset.prefetch_related(
return self.queryset.prefetch_related(
"moderated_by", "updated_by", "pending_moderation"
)
return self.extend_queryset()
def extend_queryset(self):
"""
Function to extend get_queryset when subclassing
"""
return self.my_model_queryset
......@@ -55,11 +55,13 @@ class ScholarshipSerializer(BasicModuleSerializer):
FORCE_FULL_DISPLAY = True
def my_validate(self, attrs):
def validate(self, attrs):
"""
Custom attribute validation
"""
attrs = super(ScholarshipSerializer, self).my_validate(attrs)
attrs = super().validate(attrs)
if attrs["amount_min"] is not None:
if attrs["currency"] is None:
raise serializers.ValidationError(
......
......@@ -29,7 +29,9 @@ class TaggedItemSerializer(BasicModuleSerializer):
FORCE_FULL_DISPLAY = True
def my_validate(self, attrs):
def validate(self, attrs):
attrs = super().validate(attrs)
tagged_item_validation(attrs)
return attrs
......@@ -39,8 +41,8 @@ class TaggedItemViewSet(BasicModuleViewSet):
Tagged item viewset
"""
def extend_queryset(self):
def get_queryset(self):
"""
Extend the queryset for a bit of optimization
"""
return self.my_model_queryset.prefetch_related("tag")
return super().get_queryset().prefetch_related("tag")
......@@ -33,6 +33,6 @@ class CampusTaggedItemViewSet(TaggedItemViewSet):
queryset = CampusTaggedItem.objects.all() # pylint: disable=E1101
serializer_class = CampusTaggedItemSerializer
def extend_queryset(self):
def get_queryset(self):
campus_id = self.kwargs["campus_id"]
return self.my_model_queryset.filter(campus=campus_id).distinct()
return super().get_queryset().filter(campus=campus_id).distinct()
......@@ -34,6 +34,6 @@ class CityTaggedItemViewSet(TaggedItemViewSet):
queryset = CityTaggedItem.objects.all() # pylint: disable=E1101
serializer_class = CityTaggedItemSerializer
def extend_queryset(self):
def get_queryset(self):
city_id = self.kwargs["city_id"]
return self.my_model_queryset.filter(city=city_id).distinct()
return super().get_queryset().filter(city=city_id).distinct()
......@@ -30,6 +30,6 @@ class CountryDriViewSet(BasicModuleViewSet):
queryset = CountryDri.objects.all() # pylint: disable=E1101
serializer_class = CountryDriSerializer
def extend_queryset(self):
def get_queryset(self):
country_id = self.kwargs["country_id"]
return self.my_model_queryset.filter(countries__pk=country_id).distinct()
return super().get_queryset().filter(countries__pk=country_id).distinct()
......@@ -28,6 +28,6 @@ class CountryScholarshipViewSet(ScholarshipViewSet):
queryset = CountryScholarship.objects.all() # pylint: disable=E1101
serializer_class = CountryScholarshipSerializer
def extend_queryset(self):
def get_queryset(self):
country_id = self.kwargs["country_id"]
return self.my_model_queryset.filter(countries__pk=country_id).distinct()
return super().get_queryset().filter(countries__pk=country_id).distinct()
......@@ -33,6 +33,6 @@ class CountryTaggedItemViewSet(TaggedItemViewSet):
queryset = CountryTaggedItem.objects.all() # pylint: disable=E1101
serializer_class = CountryTaggedItemSerializer
def extend_queryset(self):
def get_queryset(self):
country_id = self.kwargs["country_id"]
return self.my_model_queryset.filter(country=country_id).distinct()
return super().get_queryset().filter(country=country_id).distinct()
......@@ -30,6 +30,6 @@ class UniversityDriViewSet(BasicModuleViewSet):
queryset = UniversityDri.objects.all() # pylint: disable=E1101
serializer_class = UniversityDriSerializer
def extend_queryset(self):
def get_queryset(self):
univ_id = self.kwargs["univ_id"]
return self.my_model_queryset.filter(universities__pk=univ_id).distinct()
return super().get_queryset().filter(universities__pk=univ_id).distinct()
......@@ -32,6 +32,6 @@ class UniversityScholarshipViewSet(ScholarshipViewSet):
queryset = UniversityScholarship.objects.all() # pylint: disable=E1101
serializer_class = UniversityScholarshipSerializer
def extend_queryset(self):
def get_queryset(self):
univ_id = self.kwargs["univ_id"]
return self.my_model_queryset.filter(universities__pk=univ_id).distinct()
return super().get_queryset().filter(universities__pk=univ_id).distinct()
......@@ -37,8 +37,8 @@ class UniversitySemestersDates(BasicModule):
class UniversitySemestersDatesSerializer(BasicModuleSerializer):
def my_validate(self, attrs):
attrs = super(UniversitySemestersDatesSerializer, self).my_validate(attrs)
def validate(self, attrs):
attrs = super().validate(attrs)
s_b, s_e = attrs["spring_begin"], attrs["spring_end"]
a_b, a_e = attrs["autumn_begin"], attrs["autumn_end"]
......
......@@ -33,6 +33,6 @@ class UniversityTaggedItemViewSet(TaggedItemViewSet):
queryset = UniversityTaggedItem.objects.all() # pylint: disable=E1101
serializer_class = UniversityTaggedItemSerializer
def extend_queryset(self):
def get_queryset(self):
univ_id = self.kwargs["univ_id"]
return self.my_model_queryset.filter(university__pk=univ_id).distinct()
return super().get_queryset().filter(university__pk=univ_id).distinct()
......@@ -36,11 +36,18 @@ class UserDataSerializer(MyModelSerializer):
def get_owner_can_post_to(self, obj):
return list_user_post_permission(obj.owner)
def my_pre_save(self):
self.override_validated_data({"owner": self.user})
def do_before_save(self):
"""
For safety: enforce (for sure) that we update the model corresponding to the user/owner.
"""
super().do_before_save()
user = self.get_user_from_request()
self.override_validated_data({"owner": user})
# we try to recover the correct instance
query = UserData.objects.filter(owner=self.user)
query = UserData.objects.filter(owner=user)
if len(query) == 1:
self.instance = query[0]
......
......@@ -19,8 +19,8 @@ class ScholarshipTestCase(TestCase):
with pytest.raises(ValidationError):
attrs["amount_min"] = 200
attrs["amount_max"] = 100
ser.my_validate(attrs)
ser.validate(attrs)
attrs["amount_min"] = 100
attrs["amount_max"] = 200
ser.my_validate(attrs)
ser.validate(attrs)
......@@ -16,11 +16,11 @@ class SemesterDatesTestCase(TestCase):
def _test_attrs_error(attrs):
with pytest.raises(ValidationError):
self.ser.my_validate(attrs)
self.ser.validate(attrs)
self.ser = UniversitySemestersDatesSerializer()
_test_attrs_error(build([None, 3, None, None]))
_test_attrs_error(build([2, 3, 3, 2]))
self.ser.my_validate(build([None] * 4))
self.ser.my_validate(build([2, 3, None, None]))
self.ser.my_validate(build([2, 3, 2, 3]))
self.ser.validate(build([None] * 4))
self.ser.validate(build([2, 3, None, None]))
self.ser.validate(build([2, 3, 2, 3]))
......@@ -2,7 +2,7 @@ from .__is_member import is_member
from shared import OBJ_MODERATION_PERMISSIONS
def get_user_level(user):
def get_user_level(user) -> int:
"""
TODO unit test
"""
......
......@@ -4505,12 +4505,14 @@
"balanced-match": {
"version": "1.0.0",
"bundled": true,
"dev": true
"dev": true,
"optional": true
},
"brace-expansion": {
"version": "1.1.11",
"bundled": true,
"dev": true,
"optional": true,
"requires": {
"balanced-match": "^1.0.0",
"concat-map": "0.0.1"
......@@ -4531,12 +4533,14 @@
"concat-map": {
"version": "0.0.1",
"bundled": true,
"dev": true
"dev": true,
"optional": true
},
"console-control-strings": {
"version": "1.1.0",
"bundled": true,
"dev": true
"dev": true,
"optional": true
},
"core-util-is": {
"version": "1.0.2",
......@@ -4653,7 +4657,8 @@
"inherits": {
"version": "2.0.3",
"bundled": true,
"dev": true
"dev": true,
"optional": true
},
"ini": {
"version": "1.3.5",
......@@ -4680,6 +4685,7 @@
"version": "3.0.4",
"bundled": true,
"dev": true,
"optional": true,
"requires": {
"brace-expansion": "^1.1.7"
}
......@@ -4693,6 +4699,7 @@
"version": "2.2.4",
"bundled": true,
"dev": true,
"optional": true,
"requires": {
"safe-buffer": "^5.1.1",
"yallist": "^3.0.0"
......@@ -4804,6 +4811,7 @@
"version": "1.4.0",
"bundled": true,
"dev": true,
"optional": true,
"requires": {
"wrappy": "1"
}
......@@ -4925,6 +4933,7 @@
"version": "1.0.2",
"bundled": true,
"dev": true,
"optional": true,
"requires": {
"code-point-at": "^1.0.0",
"is-fullwidth-code-point": "^1.0.0",
......@@ -5675,7 +5684,6 @@
"version": "1.1.11",
"bundled": true,
"dev": true,
"optional": true,
"requires": {
"balanced-match": "^1.0.0",
"concat-map": "0.0.1"
......@@ -5690,8 +5698,7 @@
"code-point-at": {
"version": "1.1.0",
"bundled": true,
"dev": true,
"optional": true
"dev": true
},
"concat-map": {
"version": "0.0.1",
......@@ -5702,8 +5709,7 @@
"console-control-strings": {
"version": "1.1.0",
"bundled": true,
"dev": true,
"optional": true
"dev": true
},
"core-util-is": {
"version": "1.0.2",
......@@ -5820,8 +5826,7 @@
"inherits": {
"version": "2.0.3",
"bundled": true,
"dev": true,
"optional": true
"dev": true
},
"ini": {
"version": "1.3.5",
......@@ -5833,7 +5838,6 @@
"version": "1.0.0",
"bundled": true,
"dev": true,
"optional": true,
"requires": {
"number-is-nan": "^1.0.0"
}
......@@ -5848,7 +5852,6 @@
"version": "3.0.4",
"bundled": true,
"dev": true,
"optional": true,
"requires": {
"brace-expansion": "^1.1.7"
}
......@@ -5856,14 +5859,12 @@
"minimist": {
"version": "0.0.8",
"bundled": true,
"dev": true,
"optional": true
"dev": true
},
"minipass": {
"version": "2.3.5",
"bundled": true,
"dev": true,
"optional": true,
"requires": {
"safe-buffer": "^5.1.2",
"yallist": "^3.0.0"
......@@ -5882,7 +5883,6 @@
"version": "0.5.1",
"bundled": true,
"dev": true,
"optional": true,
"requires": {
"minimist": "0.0.8"
}
......@@ -5963,8 +5963,7 @@
"number-is-nan": {
"version": "1.0.1",
"bundled": true,
"dev": true,
"optional": true
"dev": true
},
"object-assign": {
"version": "4.1.1",
......@@ -5976,7 +5975,6 @@
"version": "1.4.0",
"bundled": true,
"dev": true,
"optional": true,
"requires": {
"wrappy": "1"
}
......@@ -6098,7 +6096,6 @@
"version": "1.0.2",
"bundled": true,
"dev": true,
"optional": true,
"requires": {
"code-point-at": "^1.0.0",
"is-fullwidth-code-point": "^1.0.0",
......
......@@ -34,40 +34,17 @@ class CustomComponentForAPI extends Component {
/////
componentDidMount() {
this.readPropsIfNeeded();
this.customComponentDidMount();
this.forceUpdate(); // bug otherwise
}
customComponentDidMount() { }
shouldComponentUpdate(nextProps, nextState) {
// Below is buggy with redux connect
// if (this.nextProps && typeof this.nextProps.visible != 'undefined' && !this.nextProps.visible) {
// // don't rerender components that won't be visible.
// return false;
// }
// if (this.props.visible === false && !this.nextProps) {
// // don't rerender components if it is still not visible
// return false;
// }
// // if (this.nextProps && typeof this.nextProps.visible != 'undefined' && this.nextProps.visible) {
// // // render if nextprops should be visible
// // return true;
// // }
return this.customShouldComponentUpdate(nextProps, nextState);
}
// eslint-disable-next-line no-unused-vars
customShouldComponentUpdate(nextProps, nextState) { return true; }
// eslint-disable-next-line no-unused-vars
componentDidUpdate(prevProps, prevState, snapshot) {
// TODO add expire date
// extends the default behavior of react to load props (api related) if needed on update.
this.readPropsIfNeeded();
this.customComponentDidUpdate(prevProps, prevState, snapshot);
}
customComponentDidUpdate() { }
// Override of the default render behaviour to wait for data to arrive.
render() {
// Override of the default react render behavior to wait for data to arrive.
// You should use `customRender` instead of `render` in your components.
if (this.checkPropsFailed()) {
return <p>Une erreur est survenue lors du téléchargement des données. Merci de recharger la page et si l'erreur persiste, merci de contacter les administrateurs du site.</p>;
}
......@@ -79,6 +56,15 @@ class CustomComponentForAPI extends Component {
return this.customRender();
}
/**
* Function to use instead of `render`.
*
* @memberof CustomComponentForAPI
*/
customRender() {
// eslint-disable-next-line no-console
console.error("Dev: you forget to define the `customRender` function that is used when rendering within a subClass of CustomComponentForAPI");
}
// End of react functions override
......@@ -270,7 +256,7 @@ class CustomComponentForAPI extends Component {
/**
* Funciton to get the city and the country of a university given a university id
* Function to get the city and the country of a university given a university id
*
* @param {number} univId
* @returns {object} Object with city and country instance of the university (main campus)
......@@ -299,7 +285,7 @@ class CustomComponentForAPI extends Component {
}
CustomComponentForAPI.propTypes = {
api: PropTypes.object
api: PropTypes.object.isRequired
};
export default CustomComponentForAPI;
......@@ -23,7 +23,9 @@ import { saveAppTheme } from "../actions/theme";
class ThemeProvider extends CustomComponentForAPI {
state = { theme: this.props.themeSavedInTheApp.theme };
customComponentDidUpdate() {
componentDidUpdate(prevProps, prevState, snapshot) {
super.componentDidUpdate(prevProps, prevState, snapshot);
if (!this.allApiDataIsReady()) {
return;
}
......
......@@ -42,9 +42,10 @@ class UnivMap extends CustomComponentForAPI {
this.updateDimensions();
}
customComponentDidMount() {
componentDidMount() {
// add an event listener to resize the map when needed
window.addEventListener("resize", this.updateDimensions.bind(this));
super.componentDidMount();
}
componentWillUnmount() {
......
......@@ -33,7 +33,9 @@ import {
*/
class PageUniversity extends CustomComponentForAPI {
customComponentDidUpdate() {
componentDidUpdate(prevProps, prevState, snapshot) {
super.componentDidUpdate(prevProps, prevState, snapshot);
if (this.props.universities.readSucceeded.readAt) {
// we have the university data
const universities = this.getReadData("universities"),
......
......@@ -66,7 +66,9 @@ class ColorTool extends CustomComponentForAPI {
}
customComponentDidUpdate() {
componentDidUpdate(prevProps, prevState, snapshot) {
super.componentDidUpdate(prevProps, prevState, snapshot);
if (!this.allApiDataIsReady()) {
return;
}
......
......@@ -136,7 +136,9 @@ class Editor extends CustomComponentForAPI {
}
}
customComponentDidUpdate() {
componentDidUpdate(prevProps, prevState, snapshot) {
super.componentDidUpdate(prevProps, prevState, snapshot);
// usefull for handling moderation
const { dataToSave } = this.props;
if (dataToSave) {
......
......@@ -8,7 +8,9 @@ class Module extends CustomComponentForAPI {
this.props.invalidateData(true);
}
customComponentDidUpdate() {
componentDidUpdate(prevProps, prevState, snapshot) {
super.componentDidUpdate(prevProps, prevState, snapshot);
this.getListPropsNeedRead()
.forEach(propName => this.performReadFromApi(propName));
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment