Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rethink rendering and responing methods in WASession #100

Closed
GoogleCodeExporter opened this issue Mar 25, 2015 · 7 comments
Closed

Rethink rendering and responing methods in WASession #100

GoogleCodeExporter opened this issue Mar 25, 2015 · 7 comments

Comments

@GoogleCodeExporter
Copy link

Do they really belong into WASession (IMHO no)?
Which ones of these do we still need?
Is there are better way to address the problem they solve?


Original issue reported on code.google.com by philippe...@gmail.com on 16 Jul 2008 at 5:23

@GoogleCodeExporter
Copy link
Author

Original comment by jfitz...@gmail.com on 21 Sep 2008 at 5:48

  • Added labels: Milestone-2.9alpha
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Looking at the 'responding' category:

#respond:onAnswer: needs to stay as far as I can see because it needs to store 
the
continuation in the Session's cache. This is a *very* confusing name, though, 
since
"answer" has a specific meaning in seaside and the first parameter sounds like 
it
should be a response. Even just #respond:continuation: would be clearer (and 
maybe
the first parameter should accept either a WAResponse or a block?).

#respond: depends on #respond:onAnswer: and so needs to stay.

#closePopup, #closePopupAndContinue, and #closePopupWithoutReloadingOpener 
depend on
#respond: and so can't really go onto the RequestContext. That said, they're 
kind of
ugly... we could just deprecate them entirely. Or... other thoughts?

#pageIntentionallyLeftBlank, at very least is a cheezy name. 
#returnEmptyResponse or
something would be more correct and could be implemented on the RequestContext
instead (there's nothing session-specific about it). It seems a bit pointless 
to add
there though, since we could just deprecate it and encourage users to use
#returnResponse: with "WARespone new"; I'm not sure how important a convenience
method it is. We could alternatively use the ResponseFactory to generate an 
"empty
response", which would allow such a response to be customized per-application if
desired. Either the session or application would then need to get the response 
and
pass it to #returnResponse:

#newSessionUrl is a bit pointless since the Application's #baseUrl should 
already
include any overridden hostname values. A "new session" is exactly the same
conceptually as simply entering the Application with no parameters, which is 
exactly
what "self application baseUrl" should be providing. I think we should just 
deprecate
this method and remove it.

All the methods in the 'redirecting' category depend on either the 
ResponseFactory or
the #respond: method and so can't really go anywhere else (well, they could all 
move
to WAApplication if desired but I don't think that's any better).

I don't see any "rendering" methods, so I assume you meant the redirecting ones?

The "expiring" methods may need to go, but that's covered by Issue 176. I also 
don't
like the "script" methods, but they're referenced by Issue 111.

Everything else looks pretty sane... WASession is looking pretty lean now.


Original comment by jfitz...@gmail.com on 4 Oct 2008 at 10:42

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

So, to summarize my suggested actions would be:

- Rename #respond:onAnswer: (maybe to #respond:continuation:). Forget my 
suggestion
about taking a block or a WAResponse... doesn't make sense.
- Implement #returnEmptyResponse in terms of the application's ResponseFactory 
and
deprecate #pageIntentionallyLeftBlank

Also, optionally do the following, though maybe someone else thinks they're 
important:
- Deprecate #closePopup* methods
- Deprecate #newSessionUrl in favour of WAApplication>>baseUrl

Comments please?

Original comment by jfitz...@gmail.com on 4 Oct 2008 at 10:46

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

I'm against deprecating the #closePopup* methods without having a replacement. 
As for
the renamings since they are mostly internal methods, that should not cause too 
much
trouble.

A general note about deprecation: it would be helpful to add in which version we
deprecated a method so that we know in which we can remove it.

Original comment by philippe...@gmail.com on 5 Oct 2008 at 10:15

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Don't we just remove all deprecated methods at the beginning of the development 
cycle
for the next major version? That's how I've always seen deprecation work. As 
soon as
you create the branch for 2.10, the first thing you do is remove the deprecated 
methods.

I just had a thought... the methods that depend on the session could still be 
moved
onto WARequestContext, if desired, as class extensions from the Session 
package. They
can still access the session from there with #session. We really might want a 
clearer
name than #respond: then... not sure.

Original comment by jfitz...@gmail.com on 5 Oct 2008 at 10:32

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Original comment by jfitz...@gmail.com on 18 Oct 2008 at 1:10

  • Changed state: Started
  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

-remove WASession>>pageIntentionallyLeftBlank
-remove WASession>>closePopupAndContinue
-move WASession>>respond:onAnswer: to WARequestContext>>respond:continuation:
(internal method, no deprecation)
-deprecate WASession>>newSessionUrl in favour of WAApplication>>baseUrl
-deprecate WASession>>closePopup in favour of
WARequestContext>>closeThisPopupAndReloadOpener
-deprecate WASession>>closePopupWithoutReloadingOpener in favour of
WARequestContext>>closeThisPopup
-deprecate WASession>>respond: in favour of 
WARequestContext>>respondAndContinue:
-deprecate WASession>>redirect in favour of WARequestContext>>redirect
-deprecate WASession>>redirectResponseFor: in favour of
WARequestContext>>redirectResponseFor:
-deprecate WASession>>redirectTo: in favour of WARequestContext>>redirectTo:
-deprecate WASession>>redirectWithMessage:delay: in favour of
WARequestContext>>redirectWithMessage:delay:
-deprecate WASession>>redirectWithCookie: in favour of
WARequestContext>>redirectWithCookie:, which no longer sets the "cookie check" 
URL
parameter
-rename WARequestContext>>returnReponse: to #respond: and update deprecation 
and senders
-implement WASession>>testSessionCookie which basically does the same as
#redirectWithCookie:



Name: Seaside-Tests-Functional-jf.8
Author: jf
Time: 18 October 2008, 6:17:24 pm
UUID: b55d16d0-0092-ae4e-b2a3-571d8ddf7c25
Ancestors: Seaside-Tests-Functional-jf.7

Name: Seaside-Squeak-Development-jf.30
Author: jf
Time: 18 October 2008, 6:18:51 pm
UUID: 1303a4bd-03d0-3342-bb1a-7509a4e80d3a
Ancestors: Seaside-Squeak-Development-lr.29

Name: Seaside-Core-jf.287
Author: jf
Time: 18 October 2008, 6:19:46 pm
UUID: e415d9b8-ee7c-de45-899c-810ec57baef5
Ancestors: Seaside-Core-jf.286

Name: Seaside-Development-jf.19
Author: jf
Time: 18 October 2008, 6:21:18 pm
UUID: 4d9f0685-99a2-cf45-8c91-e03a38503139
Ancestors: Seaside-Development-jf.18

Name: Scriptaculous-Core-jf.52
Author: jf
Time: 18 October 2008, 6:25:39 pm
UUID: e36bd88e-1ec8-f84c-8681-a5d39ba21947
Ancestors: Scriptaculous-Core-lr.51

Name: Seaside-Canvas-jf.11
Author: jf
Time: 18 October 2008, 6:26:20 pm
UUID: 42a6e8f6-7c66-f645-ad82-b28226ea9e96
Ancestors: Seaside-Canvas-pmm.10

Name: Seaside-Component-jf.15
Author: jf
Time: 18 October 2008, 6:26:50 pm
UUID: e6f6b021-c339-bc45-9d70-948adb7f7672
Ancestors: Seaside-Component-lr.14

Name: JQuery-Core-jf.7
Author: jf
Time: 18 October 2008, 6:27:23 pm
UUID: e11247d4-8bab-9d49-aa41-8b407cd0cf38
Ancestors: JQuery-Core-lr.6

Name: Seaside-Session-jf.29
Author: jf
Time: 18 October 2008, 6:28:38 pm
UUID: 547bb339-f282-2041-8a8a-2a52a1455656
Ancestors: Seaside-Session-jf.28

http://code.google.com/p/seaside/issues/detail?id=100

Cleanup responding and redirecting methods on WASession. Most went to
WARequestContext and were deprecated on WASession (a few were renamed on the 
way). A
couple were deleted.

See the issue for details.

Original comment by jfitz...@gmail.com on 18 Oct 2008 at 4:32

  • Changed state: Fixed
  • Added labels: ****
  • Removed labels: ****

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant