tnc-ba / strongTNC

BYOD TNC Database Management Tool
GNU Affero General Public License v3.0
0 stars 0 forks source link

Make session url parameter in tag inventory preselect the session #152

Closed d22 closed 10 years ago

cfaessler commented 10 years ago

depends on https://github.com/tnc-ba/strongTNC/issues/148

d22 commented 10 years ago

You can use the HashQueryObject.js, introduced with ajax paging.

d22 commented 10 years ago

Very cool addition :heart:!

Just one thing: The ignoreEvents flag is to avoid the notifications being fired, during a manually invoked set of the hash, am I right? If so, couldn't this flag be internally set when the set method is called and removed after the set is done? Otherwise there is a chance to froget to reset the flag to false... What do you think?

cfaessler commented 10 years ago

Jep this is the intended purpose of the ingoreEvents flag. Thanks for the hint, I will change it.

cfaessler commented 10 years ago

@d22 now its ready for review

d22 commented 10 years ago

I tested it and it works! There are a few small things which should be handled. See the comments in the code and the follwing points:

I guess we can merge it like this for the next pull request, as the functionality is given. However, we should create a ticket to handle the open issues on this.

@dbrgn, @cfaessler, what do you think?

dbrgn commented 10 years ago

Regarding the dates, you need to update the ajax views:

[1:24:46 PM] Danilo: der punkt ist, mit utcfromtimestamp kriegst du utc time.
[1:24:52 PM] Danilo: die musst du dann quasi zu localtime konvertieren.
[1:24:55 PM] Danilo: bin grad am suchen wie man das macht
[1:25:03 PM] Danilo: ohne dass man die zeitzone manuell angeben muss :)
[1:25:39 PM] Danilo: https://docs.djangoproject.com/en/dev/ref/utils/#django.utils.timezone.localtime
[1:26:05 PM] Danilo: "aware datetime" heisst, dass es zeitzoneninformation attached hat. das machen wir beim "utcfromtimestamp".
[1:26:22 PM] Danilo: wenn du keine zeitzone explizit angibst, nimmt er die lokale (wie in settings.py konfiguriert)
[1:26:30 PM] Danilo: alles klar? :)
dbrgn commented 10 years ago

The last two points that Jonas made are still missing. But as he said, I think we can create separate issues for them and merge this after fixing the datetime bug.

cfaessler commented 10 years ago

@dbrgn no, I don't get it. Does this mean all the other views just using "Session.time" are false and should convert time to localtime first?

dbrgn commented 10 years ago

When you "print" a date in a template, it gets converted to localtime automatically.

But when you "strftime" a date in a view, it uses the specified timezone (utc in that case). That's why there's a difference between the two dates in the template - one of them is shown directly (AFAIK), while the other one is returned via an ajax view.

cfaessler commented 10 years ago

@dbrgn Can you please review the changes?

dbrgn commented 10 years ago

The latest 2 commits look good. There's a bug in the test suite though: https://travis-ci.org/tnc-ba/strongTNC/builds/25953621

Can you verify whether it's actually a bug in the test, or a bug in the code?

dbrgn commented 10 years ago

When I click on a unique ID and then push the browser back button, the date fields are empty. Problem or not, @cfaessler?

I'd suggest: Create a separate issue for this (fill in the date fields using the timestamps from the hash) and merge!

vampy_cats