openmeetings-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ankush Mishra <ankushmish...@gmail.com>
Subject Re: Minor issue, from the GSOC work
Date Sat, 27 Oct 2018 15:53:52 GMT
Don't worry about it, no blaming here, it's possible I made the original
mistake, but oh well. At least it's fixed now. :D

Regarding tests, I'm not sure how we could handle this scenario, for now we
can do functional tests, but not sure if this is handleable due to the web
page based dependency.

Anyway, Cheers
Ankush


On Sat, Oct 27, 2018, 21:05 Maxim Solodovnik <solomax666@gmail.com> wrote:

> Hello Ankush,
>
> to be fair I don't remember if I have changed this on purpose (and don't
> remember I have changed it)
>
> The only good way I can see to prevent such situation in the future: to
> write good tests
>
> I'm trying to keep code clean and working
> But we all are humans :(
>
>
> On Sat, 27 Oct 2018 at 22:25, Ankush Mishra <ankushmishra9@gmail.com>
> wrote:
>
>> Hello Maxim,
>>
>> I just found out about this but, there's an issue based on the GSOC work,
>> not sure of when this was introduced, but based on your commit, here
>> <https://github.com/apache/openmeetings/blob/3de943d71750dc48c6e5b97322acca30849dabdb/openmeetings-web/src/main/java/org/apache/openmeetings/web/user/calendar/AppointmentDialog.java#L239>
>> from the first commit from the GSOC work merge, the order of Line 239 is
>> wrong, and could produce very erroneous behaviour.
>>
>> Simply put originally we had:
>>
>>  > getBean(AppointmentDao.class).update(a, getUserId());
>>  >      if (a.getCalendar() != null) {
>>  >           calendarPanel.updatedeleteAppointment(target,
>> CalendarDialog.DIALOG_TYPE.UPDATE_APPOINTMENT, a);
>> > }
>>
>> Where as it should have been the other way round, i.e.:
>>
>>  >      if (a.getCalendar() != null) {
>>  >           calendarPanel.updatedeleteAppointment(target,
>> CalendarDialog.DIALOG_TYPE.UPDATE_APPOINTMENT, a);
>> > }
>> > getBean(AppointmentDao.class).update(a, getUserId());
>>
>> The reason for this is that, appointment's href and etag attributes are
>> updated in the call to the calendarpanel, this change is thus not saved for
>> the future. Which can cause issues when deleting, because we may not have a
>> location to delete.
>>
>> Anyway, taking all those into consideration, I have fixed it on my fork.
>> Now, the question remains, how do we ensure such a situation does not arise
>> in the future?
>>
>> Regards,
>> Ankush
>>
>
>
> --
> WBR
> Maxim aka solomax
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message